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

Remix v1.16.0 - Unnecessary Node polyfills added for Oxygen/Workers. #6348

Closed
1 task done
frandiox opened this issue May 8, 2023 · 4 comments · Fixed by #6562
Closed
1 task done

Remix v1.16.0 - Unnecessary Node polyfills added for Oxygen/Workers. #6348

frandiox opened this issue May 8, 2023 · 4 comments · Fixed by #6562

Comments

@frandiox
Copy link
Contributor

frandiox commented May 8, 2023

What version of Remix are you using?

1.16.0

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

I think this was introduced in #5274 , and is probably related to #6280 and its linked issues/PRs. I'm still writing this issue to show that it also affects Worker environments

You can reproduce the issue when building a project with a typical config for Oxygen or CFW:

module.exports = {
  appDirectory: 'app',
  ignoredRouteFiles: ['**/.*'],
  server: './server.ts',
  serverMainFields: ['browser', 'module', 'main'],
  serverConditions: ['worker', process.env.NODE_ENV],
  serverDependenciesToBundle: 'all',
  serverModuleFormat: 'esm',
  serverPlatform: 'neutral',
  serverMinify: false, // To check the bundle later
};

Expected Behavior

The resulting server/worker bundle shouldn't contain Node polyfills. Or, at least, they shouldn't make the bundle crash when running on a Worker environment.

Actual Behavior

Many Node polyfills are added to the JS and Server bundles, thus increasing the size by ~60K non-minified. One of the polyfills is also accessing a global navigator variable, making the process exit with navigator is not defined in Worker environments:

image

image


I'm not sure if the solution is modifying this conditional and skip when serverPlatform === 'neutral', or perhaps passing different options to esbuild-plugin-polyfill-node since it seems to have a different default behavior compared to the old @esbuild-plugins/node-modules-polyfill.

@MichaelDeBoey
Copy link
Member

We just published version 1.17.1-pre.0 which includes #6562 (a possible fix). If you'd like to take it for a test run please try it out and let us know what you think!

@frandiox
Copy link
Contributor Author

frandiox commented Jun 9, 2023

Thanks! I can confirm that #6562 fixes this issue. Should we close it now or wait until 1.17.1 is released?

@MichaelDeBoey
Copy link
Member

Closed by #6562

@github-actions
Copy link
Contributor

🤖 Hello there,

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

Thanks!

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