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

fix isInteger for IE11 #389

Merged
merged 1 commit into from
Dec 13, 2018
Merged

fix isInteger for IE11 #389

merged 1 commit into from
Dec 13, 2018

Conversation

JMounier
Copy link
Contributor

@JMounier JMounier commented Dec 11, 2018

Fixes usage of Number.isInteger in lib/internal/streams/state.js

TypeError: Object doesn't support property or method 'isInteger'
       at getHighWaterMark (eval code:12:5)
  • add new integerIE11 rule in build/files.js
  • add dev dependency to babel-plugin-transform-optional-catch-binding (so that anyone can run ./build/build.js)

Fix #390

package.json Outdated
@@ -29,6 +29,7 @@
"babel-plugin-transform-es2015-spread": "^6.22.0",
"babel-plugin-transform-es2015-template-literals": "^6.8.0",
"babel-plugin-transform-inline-imports-commonjs": "^1.2.0",
"babel-plugin-transform-optional-catch-binding": "^7.0.0-alpha.16",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really confident in depending on alpha code. Is there a stable version of this transform?

Copy link
Member

Choose a reason for hiding this comment

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

You'd probably have to migrate everything to babel 7 stable (under the @babel/ namespace. This tool can help: https://github.com/babel/babel-upgrade

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any stable version on npm and the latest one is already one year old.

Anyway this package is only necessary to build readable-stream from scratch. And so is not relevant to the fix.

I'll just drop the commit adding the dependency and create an issue (#392) to upgrade to babel 7 stable as @targos said.

@JMounier JMounier mentioned this pull request Dec 13, 2018
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.

None yet

3 participants