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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Guard against process not being defined #911

Merged
merged 1 commit into from
May 14, 2021

Conversation

jhnns
Copy link
Contributor

@jhnns jhnns commented May 14, 2021

Hi all 馃憢

What:

Fixes

ReferenceError: process is not defined
    at eval (webpack-internal:///./node_modules/@testing-library/react/dist/@testing-library/react.esm.js:458:1)

when @testing-library/react is being used with webpack 5 and karma.

Why:

Webpack 5 doesn't shim Node's process object by default anymore. Only instances of process.env.NODE_ENV are replaced statically. This means that unguarded checks of process.env will crash in browser environments.

How:

I was wondering if there should also be a check in dont-cleanup-after-each.js. I decided against it since this would need to define a global process object in order to define the env variable. After all, it's the app developer's responsibility to provide a global process object in this case.

Checklist:

  • Documentation added to the
    docs site N/A
  • Tests N/A
  • Typescript definitions updated N/A
  • Ready to be merged

Webpack 5 doesn't shim Node's process object by default anymore. Only instances of process.env.NODE_ENV are replaced statically. This means that unguarded checks of process.env will crash in browser environments (such as Karma).
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a7c2a78:

Sandbox Source
React Configuration
react-testing-library-examples Configuration

@@ -5,7 +5,7 @@ import {cleanup} from './pure'
// this ensures that tests run in isolation from each other
// if you don't like this then either import the `pure` module
// or set the RTL_SKIP_AUTO_CLEANUP env variable to 'true'.
if (!process.env.RTL_SKIP_AUTO_CLEANUP) {
if (typeof process === "undefined" || !process.env?.RTL_SKIP_AUTO_CLEANUP) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does your build setup support optional chaining? I hope that's fine 馃榿

Copy link
Member

Choose a reason for hiding this comment

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

Should be transpiled loosely.

@codecov
Copy link

codecov bot commented May 14, 2021

Codecov Report

Merging #911 (a7c2a78) into main (fe16376) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #911   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          140       140           
  Branches        26        26           
=========================================
  Hits           140       140           
Impacted Files Coverage 螖
src/index.js 100.00% <100.00%> (酶)

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update fe16376...a7c2a78. Read the comment docs.

@eps1lon eps1lon added the bug Something isn't working label May 14, 2021
@eps1lon eps1lon changed the title Fix 'process is not defined' in browser environments (webpack 5 + karma) fix: Guard against process not being defined May 14, 2021
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Thanks!

@eps1lon eps1lon merged commit 8a1c8e9 into testing-library:main May 14, 2021
@eps1lon
Copy link
Member

eps1lon commented May 14, 2021

@all-contributors Add @jhnns for code

@allcontributors
Copy link
Contributor

@eps1lon

I've put up a pull request to add @jhnns! 馃帀

@github-actions
Copy link

馃帀 This PR is included in version 11.2.7 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

swissspidy added a commit to GoogleForCreators/web-stories-wp that referenced this pull request Jul 2, 2021
JoviDeCroock pushed a commit to testing-library/preact-testing-library that referenced this pull request Jan 18, 2023
Webpack 5 doesn't shim Node's `process` object by default anymore. Only instances of `process.env.NODE_ENV` are replaced statically. This means that unguarded checks of `process.env` will crash in browser environments (such as Karma).

This is a port of testing-library/react-testing-library#911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants