Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(remix-dev/vite): allow custom NODE_ENV for vite dev #7980

Merged

Conversation

hi-ogawa
Copy link
Contributor

@hi-ogawa hi-ogawa commented Nov 12, 2023

Closes: #7918

This can be achieved by

  • set skipEnvCheck: true for react-refresh/babel plugin, and
  • update LiveReload to check viteCommand = "serve" instead of NODE_ENV = "development"

I initially thought the error "React Refresh Babel transform should only be enabled in development environment." only manifests when remix plugin is (unintentionally) loaded on vitest, but I found that this can also happen, for example, when doing NODE_ENV=test vite dev.
So, I think it makes sense to disable this check (and also allow HMR) for other NODE_ENV as well since it looks like too-opinionated restriction. Also remix itself applies this transform only on "serve" command, so such warning is irrelevant.

Also note that this is yet-another compatibility with @vitejs/plugin-react, which also uses skipEnvCheck: true:
https://github.com/vitejs/vite-plugin-react/blob/38b3cece51644f3d4ba78e2352717b5077bd4637/packages/plugin-react/src/index.ts#L196-L199

Testing Strategy:

I didn't add a test case since it felt a little redundant. But, to verify the behavior, I locally checked vite-dev-test.ts still passes after manually modifying NODE_ENV: "test" here:

devProc = spawn(nodeBin, [viteBin, "dev"], {
cwd: projectDir,
env: process.env,
stdio: "pipe",
});

- env: process.env,
+ env: { ...process.env, NODE_ENV: "test" },

Copy link

changeset-bot bot commented Nov 12, 2023

🦋 Changeset detected

Latest commit: 0269019

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@remix-run/dev Patch
create-remix Patch
remix Patch
@remix-run/architect Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/css-bundle Patch
@remix-run/deno Patch
@remix-run/eslint-config Patch
@remix-run/express Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/serve Patch
@remix-run/server-runtime Patch
@remix-run/testing Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@hi-ogawa hi-ogawa changed the title fix(vite): set skipEnvCheck for react-refresh/babel plugin fix(remix-dev/vite): set skipEnvCheck for react-refresh/babel plugin Nov 12, 2023
@hi-ogawa hi-ogawa changed the title fix(remix-dev/vite): set skipEnvCheck for react-refresh/babel plugin fix(remix-dev/vite): set skipEnvCheck for react-refresh/babel plugin to allow any NODE_ENV Nov 12, 2023
@hi-ogawa hi-ogawa changed the title fix(remix-dev/vite): set skipEnvCheck for react-refresh/babel plugin to allow any NODE_ENV fix(remix-dev/vite): allow custom NODE_ENV for vite dev Nov 12, 2023
@hi-ogawa hi-ogawa marked this pull request as ready for review November 12, 2023 08:28
Copy link
Member

@markdalgleish markdalgleish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Copy link
Contributor

🤖 Hello there,

We just published version 2.3.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Copy link
Contributor

🤖 Hello there,

We just published version 2.3.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants