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

[DataGrid] ReferenceError: process is not defined #836

Closed
matandro opened this issue Jan 11, 2021 · 10 comments · Fixed by #1027
Closed

[DataGrid] ReferenceError: process is not defined #836

matandro opened this issue Jan 11, 2021 · 10 comments · Fixed by #1027
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@matandro
Copy link

matandro commented Jan 11, 2021

I could not reproduce the error with codesandbox even with the same package version. The same code runs smooth on alpha.8. I am not sure what is the exact version where it breaks.
Code is built using webpack with babel.

I receive an index-esm.js?:formatted:2044 Uncaught ReferenceError: process is not defined Error that is targeted to node_modules/@material-ui/x-grid/dist/index-esm.js. The error line is:`

On = void 0 !== process.env.EXPERIMENTAL_ENABLED && _r() && window.localStorage.getItem("EXPERIMENTAL_ENABLED") ? "true" === window.localStorage.getItem("EXPERIMENTAL_ENABLED") : "true" === process.env.EXPERIMENTAL_ENABLED;

Relevant packges list:

"@material-ui/core": "^4.11.2",
"@material-ui/icons": "^4.11.2",
"@material-ui/lab": "^4.0.0-alpha.57",
"@material-ui/x-grid": "^4.0.0-alpha.15",
"react": "^17.0.1",
"react-dom": "^17.0.1",

EDIT:
I checked each version and the error pops up in alpha 11 (up to alpha 10 I don't see the error)

@matandro matandro added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 11, 2021
@oliviertassinari oliviertassinari changed the title [X-Grid] Error upgrading from xgrid alpha.8 to alpha.15 [XGrid] Error upgrading from xgrid alpha.8 to alpha.15 Jan 13, 2021
@oliviertassinari oliviertassinari changed the title [XGrid] Error upgrading from xgrid alpha.8 to alpha.15 [DataGrid] Error upgrading from xgrid alpha.8 to alpha.15 Jan 13, 2021
@oliviertassinari oliviertassinari added component: data grid This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 13, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 13, 2021

@matandro Thanks for the report. I suspect that your bundling environment is only configured to handle process.env.NODE_ENV correctly. Do you have a reproduction? It would greatly help us understand what's going on. Once we have a reproduction, we could test this potential solution:

diff --git a/packages/grid/_modules_/grid/constants/envConstants.ts b/packages/grid/_modules_/grid/constants/envConstants.ts
index b15fb77..165492d 100644
--- a/packages/grid/_modules_/grid/constants/envConstants.ts
+++ b/packages/grid/_modules_/grid/constants/envConstants.ts
@@ -18,15 +18,16 @@ import { localStorageAvailable } from '../utils/utils';
 // Developers (users) are discouraged to enable the experimental feature by setting the EXPERIMENTAL_ENABLED env.
 // Instead, prefer exposing experimental APIs, for instance, a prop or a new `unstable_` module.

-let experimentalEnabled;
+let experimentalEnabled = false;

 if (
+  typeof process !== 'undefined' &&
   process.env.EXPERIMENTAL_ENABLED !== undefined &&
   localStorageAvailable() &&
   window.localStorage.getItem('EXPERIMENTAL_ENABLED')
 ) {
   experimentalEnabled = window.localStorage.getItem('EXPERIMENTAL_ENABLED') === 'true';
-} else {
+} else if (typeof process !== 'undefined') {
   experimentalEnabled = process.env.EXPERIMENTAL_ENABLED === 'true';
 }

@matandro
Copy link
Author

matandro commented Jan 13, 2021

I will try to implement a small test case. For now I was reading about this issue (seems to be common).
Webpack 5 no longer adds the process.env variable, to overcome the issue you can add a plugin to force it's creation:

  1. install npm process
  2. To webpack.config.js add
const webpack = require('webpack');

module.export = {
  plugins: [
    new webpack.ProvidePlugin({
      process: 'process/browser',
    }),
  ],
  // ... rest of the module
}

See more at https://webpack.js.org/migrate/5/
In general it seems the suggestion is not to use process at all, not sure why this change was made.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 13, 2021

@matandro Awesome great, it seems to confirm that the proposed diff would cover your problem.

@oliviertassinari oliviertassinari removed the status: waiting for author Issue with insufficient information label Jan 13, 2021
@oliviertassinari oliviertassinari changed the title [DataGrid] Error upgrading from xgrid alpha.8 to alpha.15 [DataGrid] Error upgrading from xgrid alpha.10 to alpha.11 Jan 13, 2021
@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jan 13, 2021
@leontastic
Copy link
Contributor

I have this issue too. I'm using esbuild for bundling.

Even when I define process.env.EXPERIMENTAL_ENABLED in my bundler, I still get this runtime error. I can run process.env.EXPERIMENTAL_ENABLED and get true at runtime in my own bundle output, but when I require @material-ui/data-grid this error still occurs at runtime.

Looking at my build output I see the following two references (the only references in the whole project!) to process.env:

BI=(process&&process.env&&"true")!==void 0&&uP()&&window.localStorage.getItem("EXPERIMENTAL_ENABLED")?window.localStorage.getItem("EXPERIMENTAL_ENABLED")==="true":(process&&process.env&&"true")==="true";

I'm assuming this is because the index-esm.js build output from this library actually includes the process && process.env. From index-esm.js:

ta=void 0!==(process && process.env && process.env.EXPERIMENTAL_ENABLED)&&Wt()&&window.localStorage.getItem("EXPERIMENTAL_ENABLED")?"true"===window.localStorage.getItem("EXPERIMENTAL_ENABLED"):"true"===(process && process.env && process.env.EXPERIMENTAL_ENABLED);

The bundler you are using to produce index-esm.js seems to replace process.env.EXPERIMENTAL_ENABLED !== undefined with 0!==(process && process.env && process.env.EXPERIMENTAL_ENABLED) in the final build output.

Hence, users' bundlers replace process.env.EXPERIMENTAL_ENABLED with "true" but leave process and process.env untouched. This means process must actually exist in the browser runtime for users to run the bundled output without encountering the ReferenceError at runtime. This is why the ProvidePlugin must be used to provide a process object with Webpack builds, and the ReferenceError still happens using esbuild.

This doesn't seem to happen with NODE_ENV. All references to process.env.NODE_ENV === "production" are replaced with "production"!==process.env.NODE_ENV in the final index-esm.js output, which is replaced with "production"!=="development" or evaluated to true/false by users' bundlers. I'm guessing the comparison process.env.EXPERIMENTAL_ENABLED !== undefined could probably be rewritten in a way that your bundler doesn't split up process.env.EXPERIMENTAL_ENABLED into process && process.env && process.env.EXPERIMENTAL_ENABLED to avoid the issue, but I honestly don't know how it should be written.

I don't think @oliviertassinari's suggested diff will fix the issue because the process.env.EXPERIMENTAL_ENABLED !== undefined expression will probably still build into process && process.env && process.env.EXPERIMENTAL_ENABLED even if you add typeof process !== "undefined" && before it.

After doing so much research into this issue, I agree with @matandro it would probably be better to do away with this flag in general, and hide experimental features behind another version tag instead. That way, developers who want experimental features can get them simply by checking out a different branch or version, and you can avoid issues related to bundlers' syntax transform rules entirely. Or if you can figure out a way to rewrite process.env.EXPERIMENTAL_ENABLED !== undefined without the bundler splitting the expression up, maybe that would be a sufficient solution.

Either way, this bug makes the library completely unusable for me since I don't have a readily-available Provider plugin for esbuild to polyfill process, and frankly speaking I don't really want to add hacks to my code just to accomodate this experimental feature flag. Will just check out a pre-alpha version instead.

@oliviertassinari Bless the work you did, this library is still pretty great.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 10, 2021

@leontastic Thanks for looking into it.

Under https://unpkg.com/@material-ui/data-grid@4.0.0-alpha.19/dist/index-esm.js I see:

let ta;
ta = void 0 !== process.env.EXPERIMENTAL_ENABLED && Wt() &&
  window.localStorage.getItem("EXPERIMENTAL_ENABLED")
    ? "true" === window.localStorage.getItem("EXPERIMENTAL_ENABLED")
    : "true" === process.env.EXPERIMENTAL_ENABLED;

I'm pretty sure that the proposed fix would work. However, we could try it out with a pull request, and test it instantly with codesandbox-ci before merging. Do you want to give it a try? :)

@oliviertassinari oliviertassinari changed the title [DataGrid] Error upgrading from xgrid alpha.10 to alpha.11 [DataGrid] ReferenceError: process is not defined Feb 10, 2021
@lnorton89
Copy link

I've tried every workaround I've found--using ProvidePlugin and other ways to make process available via my Webpack config-- and cannot get this error to stop.

I checked each version and the error pops up in alpha 11 (up to alpha 10 I don't see the error)

This also does not seem to be true for me. I've tried every Alpha version down to 9 and they still error out. Is there a reason we're going against Webpacks own advice not to use process?

process.env is Node.js specific and should be avoided in frontend code.

@leontastic
Copy link
Contributor

leontastic commented Feb 10, 2021

@oliviertassinari I put your diff into a PR so you can try: #1027

BUT I think your fix would break the feature flag itself.

Even if process.env.EXPERIMENTAL_ENABLED is defined by users' bundler, if we assume process remains undefined then typeof process !== 'undefined' still evaluates to false, so both of the code paths in envConstants.ts will be skipped, hence experimentalEnabled will always remain false. Might as well just export EXPERIMENTAL_ENABLED = false in that case.

A full-text search of the entire project reveals that this flag is not even being used! Personally I would remove this flag. If that's not an option, I would set it to a string so that the bundler doesn't split up the process.env.EXPERIMENTAL_ENABLED !== undefined check into process && process.env .... I think that's why checks against process.env.NODE_ENV === 'production' don't seem to result in the issue we are seeing here.

@leontastic
Copy link
Contributor

Alternative PR removing this file (should cause 0 difference since the flag isn't being used anyway): #1028

Consider merging this PR in and re-adding an experimental flag when you actually introduce experimental features to hide. This way you can test both the experimental feature and the correctness of this flag end-to-end instead of needing to reason with the code.

@leontastic
Copy link
Contributor

leontastic commented Feb 10, 2021

@oliviertassinari I tested both PRs from codesandbox and they both work – No more ReferenceError. I also did a text-search check for process.env.EXPERIMENTAL_ENABLED and it looks like your fix actually did prevent it from getting split up into process && process.env && ... in the bundle output.

I don't know if the actual flag will work though because typeof process !== 'undefined' might return false even if process.env.EXPERIMENTAL_ENABLED is defined by my bundler. In this case the EXPERIMENTAL_ENABLED constant would always be false no matter what I set it to.

Which PR you merge is up to you! Thanks for your hard work.

@oliviertassinari
Copy link
Member

@leontastic perfect 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants