Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip Oxygen checks when @shopify/remix-oxygen is not installed #1137

Merged
merged 7 commits into from
Jul 31, 2023

Conversation

frandiox
Copy link
Contributor

Related to #1122

This PR skips Oxygen requirements we had for the config when not deploying to Oxygen.

I've also renamed config.ts to remix-config.ts to make it more obvious since we have another file called shopify-config.ts.

@frandiox frandiox requested review from blittle, juanpprieto and a team July 24, 2023 11:23
@blittle blittle changed the base branch from 2023-04 to 2023-07 July 26, 2023 20:25
@frandiox frandiox merged commit 1015f17 into 2023-07 Jul 31, 2023
10 checks passed
@frandiox frandiox deleted the fd-oxygen-checks branch July 31, 2023 13:11
FrcPpe pushed a commit to FrcPpe/hydrogen that referenced this pull request Aug 13, 2023
…pify#1137)

* Rename config file

* Extract assertOxygenChecks

* Assert Oxygen checks only in build and dev commands

* Only assert Oxygen checks when remix-oxygen is installed

* Remove type fixes for old versions of Remix

* Changesets
@ascorbic
Copy link

ascorbic commented Nov 3, 2023

@frandiox Hey. I'm updating our template and it looks like these checks are still being triggered when @shopify/remix-oxygen is not installed, because @shopify/remix-oxygen is also a dependency of @shopify/cli-hydrogen. Would it be possible to instead check if @shopify/remix-oxygen is in the package.json, so it only triggers for explicitly-installed packages?

@frandiox
Copy link
Contributor Author

frandiox commented Nov 6, 2023

Hi! Forgot to reply earlier today but here's a PR that removes @shopify/remix-oxygen from our peer deps. I think it was a mistake to add it as a peer dep in the first place. This should fix your issue. If it doesn't then I'll change it to check package.json instead 👍

Edit: @ascorbic You can try with @shopify/cli-hydrogen@0.0.0-next-8fce70d-20231107025821 to test but I think it should work. Now we only have the root project depending on the Oxygen adapter (direct dependency):

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants