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

Use globalThis (with fallback if needed) instead of global to enable esbuild support #11569

Merged
merged 2 commits into from
Jun 15, 2021
Merged

Use globalThis (with fallback if needed) instead of global to enable esbuild support #11569

merged 2 commits into from
Jun 15, 2021

Conversation

JakeChampion
Copy link
Contributor

@JakeChampion JakeChampion commented Jun 14, 2021

Summary

The motivation for this pull-request is to get the jest packages such as pretty-format to work with esbuild when targeting the browser platform. This is because @testing-library/dom uses pretty-format and I am trying to use that package as part of my testing setup.

By using global, the package pretty-format can not be bundled for the browser by esbuild because global is not defined in a browser environment.

Switching to globalThis, with a polyfill for older environments, allows pretty-format (and hopefully other packages) to be bundled by esbuild --platform browser.

Test plan

The change does not modify any logic of Jest's packages, it only adds a polyfill for globalThis.

A way to test this change will solve my original problem would be to build the project and then attempt to bundle pretty-format with esbuild --bundle --target=es2020 --platform=browser and run the bundle in a browser environment.

Before

Building from master branch:

~/Code/jest
❯ yarn build
❯ yarn build
 Building packages 
 Building TypeScript definition files 
 Successfully built TypeScript definition files 
 Validating TypeScript definition files 
 Successfully validated TypeScript definition files 
❯ esbuild packages/pretty-format/build/index.js --bundle --target=es2020 --platform=browser | pbcopy

Running the bundle in the browser gives an Error Uncaught ReferenceError: global is not defined
image

After

Building from globalthis branch:

❯ git switch -
Switched to branch 'globalthis'
❯ yarn build
 Building packages
 Building TypeScript definition files
 Successfully built TypeScript definition files
 Validating TypeScript definition files
 Successfully validated TypeScript definition files
❯ esbuild packages/pretty-format/build/index.js --bundle --target=es2020 --platform=browser | pbcopy

Running the bundle in the browser gives no error:
image

By using `global`, the package `pretty-format` can not be bundled for the browser by `esbuild` because `global` is not defined in a browser environment.

Switching to `globalThis`, with a polyfill for older environments, allows `pretty-format` (and hopefully other packages) to be bundled by `esbuild --platform browser`.
@codecov-commenter
Copy link

Codecov Report

Merging #11569 (6de8504) into master (91b94c8) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11569      +/-   ##
==========================================
- Coverage   69.00%   69.00%   -0.01%     
==========================================
  Files         312      312              
  Lines       16331    16331              
  Branches     4730     4730              
==========================================
- Hits        11270    11269       -1     
- Misses       5033     5034       +1     
  Partials       28       28              
Impacted Files Coverage Δ
packages/expect/src/utils.ts 95.58% <0.00%> (-0.56%) ⬇️

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 91b94c8...6de8504. Read the comment docs.

@JakeChampion JakeChampion marked this pull request as ready for review June 14, 2021 22:17
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this pull request Jun 15, 2021
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

thanks! Could you add an entry to the changelog as well?

@SimenB SimenB changed the title Use globalThis (with polyfill if needed) instead of global to enable esbuild support Use globalThis (with fallback if needed) instead of global to enable esbuild support Jun 15, 2021
@SimenB SimenB merged commit 7be63ef into jestjs:master Jun 15, 2021
@JakeChampion JakeChampion deleted the globalthis branch June 15, 2021 11:18
@SimenB
Copy link
Member

SimenB commented Jun 22, 2021

https://github.com/facebook/jest/releases/tag/v27.0.5

JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this pull request Jun 28, 2021
We are not using any of the features which were removed or changed in the new version.

The new version also removes the need for the hack.js file we needed to use this is because, the package is using the latest major versio of pretty-format, which contains this update jestjs/jest#11569
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this pull request Jun 30, 2021
We are not using any of the features which were removed or changed in the new version.

The new version also removes the need for the hack.js file we needed to use this is because, the package is using the latest major versio of pretty-format, which contains this update jestjs/jest#11569
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this pull request Jul 1, 2021
We are not using any of the features which were removed or changed in the new version.

The new version also removes the need for the hack.js file we needed to use this is because, the package is using the latest major versio of pretty-format, which contains this update jestjs/jest#11569
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this pull request Jul 6, 2021
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this pull request Jul 6, 2021
We are not using any of the features which were removed or changed in the new version.

The new version also removes the need for the hack.js file we needed to use this is because, the package is using the latest major versio of pretty-format, which contains this update jestjs/jest#11569
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants