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

Add polyfill for BigInt #70

Closed
wants to merge 1 commit into from

Conversation

lukasIO
Copy link

@lukasIO lukasIO commented May 11, 2023

Thanks first of all for this project, it's exactly what I was looking for!

This PR simply adds an option to polyfill BigInt to the rules.

@robatwilliams
Copy link
Owner

I didn't think BigInt could be polyfilled properly. This article agrees: https://javascript.info/bigint#polyfills

What are you using to polyfill it?

@robatwilliams
Copy link
Owner

Made me think - raised #71

@lukasIO
Copy link
Author

lukasIO commented May 11, 2023

A bit more context:
I'm running the compat check on the bundled output of a TypeScript project.
A third party dependency of the lib is doing some checks like

if(globalThis.BigInt !== undefined) {
  // ... proceeds to reference BigInt
}

that should be safe (as long as globalThis is actually polyfilled), but the es lint plugin complains.
So I'm trying to work around it.
Totally agree though that it's not really ideal as that would mask other occurences of BigInt to be wrongfully marked as polyfilled 🤔

@robatwilliams
Copy link
Owner

I see.

The problem of feature checks is a general one, but for all non-syntax features it can be worked around by stating that you have a polyfill (even if you don't actually). Except for things that aren't properly (or at all) polyfillable like BigInt here and probably ES2017 Atomics.

It might be possible to implement something here to guess if an expression looks like a feature test. We have the AST node there. This feels like the proper way to solve the wider problem, which I'd like to try.

In the meantime perhaps you could use your code changes here via patch-package ?

@lukasIO
Copy link
Author

lukasIO commented May 12, 2023

To have proper support for detecting feature tests sounds ideal!

Yeah, I'll work around it with a forked version for now, but very excited to see support for feature checks!

Thanks for your feedback, will close this PR.

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