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

[Refactor] use a global variable to get the original BigInt instead of a global property #11

Merged
merged 1 commit into from
Apr 19, 2022

Conversation

triptate
Copy link
Contributor

not all bundlers polyfill the node global variable. this change makes the library work universally in node or the browser, whether global is polyfilled or not.

a discussion of the issue for a similar library is here: inspect-js/has-symbols#11

@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #11 (a7ccfac) into main (62d31e7) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #11   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            6         6           
  Branches         4         5    +1     
=========================================
  Hits             6         6           
Impacted Files Coverage Δ
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 62d31e7...a7ccfac. Read the comment docs.

@ljharb
Copy link
Member

ljharb commented Apr 19, 2022

All bundlers that aren't broken do - a node module bundler's job is to provide node globals and shims for node builtins whenever possible.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Same comment as in inspect-js/has-symbols#11 (comment): stop using a bundler that's broken. However, this is still equally correct, so I suppose it's fine.

@ljharb ljharb force-pushed the use-universal-accessor branch from 57ad9a1 to 7d958bb Compare April 19, 2022 21:00
@ljharb ljharb changed the title use a universal accessor for the original BigInt [Refactor] use a global variable to get the original BigInt instead of a global property Apr 19, 2022
@triptate
Copy link
Contributor Author

i don't really have a horse in this fight, but here is the perspective from the Vite team: vitejs/vite#5912 (comment)

@ljharb
Copy link
Member

ljharb commented Apr 19, 2022

I realize that, and I'd strongly encourage people to avoid a bundler with that attitude like the plague. A node module bundler that does not polyfill node globals is not a node module bundler.

@ljharb ljharb force-pushed the use-universal-accessor branch from 7d958bb to a7ccfac Compare April 19, 2022 21:18
@ljharb ljharb merged commit a7ccfac into inspect-js:main Apr 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants