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

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

@JakeChampion JakeChampion force-pushed the globalthis branch 7 times, most recently from 3517c12 to b7c1c8a Jun 14, 2021
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-commenter codecov-commenter commented Jun 14, 2021

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 Jun 14, 2021
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this issue Jun 15, 2021
SimenB
SimenB approved these changes Jun 15, 2021
Copy link
Collaborator

@SimenB SimenB left a comment

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

SimenB
SimenB approved these changes Jun 15, 2021
@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 facebook:master Jun 15, 2021
6 of 9 checks passed
@JakeChampion JakeChampion deleted the globalthis branch Jun 15, 2021
@SimenB
Copy link
Collaborator

@SimenB SimenB commented Jun 22, 2021

JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this issue 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 facebook/jest#11569
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this issue 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 facebook/jest#11569
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this issue 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 facebook/jest#11569
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this issue Jul 6, 2021
JakeChampion added a commit to Financial-Times/o-autocomplete that referenced this issue 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 facebook/jest#11569
@github-actions
Copy link

@github-actions github-actions bot commented Jul 23, 2021

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
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants