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

Update babel-plugin-proposal-unicode-property-regex for Unicode v12 #9636

Merged
merged 2 commits into from Mar 5, 2019

Conversation

@mathiasbynens
Copy link
Contributor

mathiasbynens commented Mar 5, 2019

@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Mar 5, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/10386/

@buildsize

This comment has been minimized.

Copy link

buildsize bot commented Mar 5, 2019

File name Previous Size New Size Change
babel-preset-env.js 2.04 MB 2.04 MB 0 bytes (0%)
babel-preset-env.min.js 1.09 MB 1.09 MB 0 bytes (0%)
babel.js 2.74 MB 3.63 MB 913.56 KB (33%)
babel.min.js 1.53 MB 2.2 MB 689.96 KB (44%)
@mathiasbynens mathiasbynens force-pushed the unicode-12 branch from 1c4b79b to b845958 Mar 5, 2019
@mathiasbynens

This comment has been minimized.

Copy link
Contributor Author

mathiasbynens commented Mar 5, 2019

Where does the babel.js increase of 912.83 KB come from? Any ideas?

The new Unicode data takes less than a KB (and that's uncompressed). Compare https://bundlephobia.com/result?p=regenerate-unicode-properties@7.0.0 vs. https://bundlephobia.com/result?p=regenerate-unicode-properties@8.0.0.

@xtuc
xtuc approved these changes Mar 5, 2019
@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Mar 5, 2019

I'm investigating the size regression

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Mar 5, 2019

Before:
schermata da 2019-03-05 17-24-37

After:
schermata da 2019-03-05 17-28-36


EDIT: For some reason the top-level version of regexpu-core is 4.4.0, so lerna can't hoist regexpu-core@4.5.2. I have manually updated the lock file.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Mar 5, 2019

Fixed
schermata da 2019-03-05 18-00-00

@nicolo-ribaudo nicolo-ribaudo merged commit 349c0d4 into master Mar 5, 2019
7 of 8 checks passed
7 of 8 checks passed
Is version commit Is version commit
Details
On master branch On master branch
Details
babel/repl REPL preview is available
Details
buildsize Significant change of babel.min.js up by 6.19 KB (0.08%)
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 87.04% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@nicolo-ribaudo nicolo-ribaudo deleted the unicode-12 branch Mar 5, 2019
@danez

This comment has been minimized.

Copy link
Member

danez commented Mar 5, 2019

Should we also update unicode in the parser and regenerate the identifier regexes?

Okay I missed the other PR.

@danez

This comment has been minimized.

Copy link
Member

danez commented Mar 5, 2019

This makes babel-standalone fail on node 6

WARNING in ./node_modules/regenerate-unicode-properties/Binary_Property/Grapheme_Base.js
Module build failed: RangeError: Maximum call stack size exceeded
    at Object.needsParens (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/node/index.js:112:10)
    at Generator.print (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/printer.js:287:25)
    at Generator.MemberExpression (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/generators/expressions.js:261:8)
    at withSource (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/printer.js:299:22)
    at Buffer.withSource (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/buffer.js:185:28)
    at Generator.withSource (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/printer.js:178:15)
    at Generator.print (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/printer.js:298:10)
    at Generator.CallExpression (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/generators/expressions.js:177:8)
    at withSource (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/printer.js:299:22)
    at Buffer.withSource (/home/travis/build/babel/babel/node_modules/@babel/generator/lib/buffer.js:185:28)
 @ ./node_modules/regenerate-unicode-properties ^\.\/.*\.js$
 @ ./node_modules/regexpu-core/rewrite-pattern.js
 @ ./packages/babel-plugin-proposal-unicode-property-regex/lib/index.js
 @ ./packages/babel-standalone/src/index.js

It does not fail travis unfortunately, which we should fix.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Mar 5, 2019

This looks like a bug with Babel itself? 🤔
Maybe it is #8278 (comment)

@mathiasbynens

This comment has been minimized.

Copy link
Contributor Author

mathiasbynens commented Mar 6, 2019

Was about to reference #8278 here myself.

mathiasbynens added a commit to mathiasbynens/regenerate-unicode-properties that referenced this pull request Mar 6, 2019
mathiasbynens added a commit to mathiasbynens/regexpu-core that referenced this pull request Mar 6, 2019
@mathiasbynens

This comment has been minimized.

Copy link
Contributor Author

mathiasbynens commented Mar 6, 2019

I just published regenerate-unicode-properties@8.0.1 which changes the generated output to avoid deeply-nested ASTs. regexpu-core@4.5.3 has the updated dependency.

mathiasbynens added a commit that referenced this pull request Mar 6, 2019
yarn upgrade regexpu-core@4.5.3

This should resolve the issue @danez mentions here:
#9636 (comment)
mathiasbynens added a commit that referenced this pull request Mar 6, 2019
yarn upgrade regexpu-core@4.5.3

This should resolve the issue @danez mentions here:
#9636 (comment)
mathiasbynens added a commit that referenced this pull request Mar 7, 2019
yarn upgrade regexpu-core@4.5.3

This should resolve the issue @danez mentions here:
#9636 (comment)
mathiasbynens added a commit that referenced this pull request Mar 7, 2019
This should resolve the issue @danez mentions here:
#9636 (comment)
@lock lock bot added the outdated label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.