assert,deps,repl: fix repl and assert not being able to handle language features properly #27400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds bigint, class-fields, numeric-separators, static-class features, private class methods and fields as dependency. That way it's possible to use these in combination with acorn to parse these language features. This also removes a couple of files that were not necessary for Node.js to reduce the code base.
This adds new language features to acorn. Otherwise BigInt and other input would not be parsed correct and would not result in nice error messages when using simple assert.
This adds stage-3 language features to acorn so that it's possible to parse these features when using top level await in the REPL.
This adds stage-3 language features to acorn so that the REPL is able to parse these features properly. Otherwise these would cause SyntaxErrors.
|
@BridgeAR Take a look at nodejs/ecmascript-modules#69. I was working around the lack of |
|
@GeoffreyBooth I suggest that you add the necessary Plugin to your PR but I am not sure if it's necessary for |
This comment has been minimized.
This comment has been minimized.
BridgeAR
added
assert
jasnell
reviewed
Apr 25, 2019
|
@nodejs/tsc PTAL about the licensing questions (see #27400 (comment)). @nodejs/assert @nodejs/repl PTAL |
|
My suggestion regarding the license:
|
targos
reviewed
Apr 26, 2019
|
@targos I incorporated your suggestion. |
|
Can you run |
This comment has been minimized.
This comment has been minimized.
BridgeAR
added a commit
to BridgeAR/node
that referenced
this pull request
Apr 30, 2019
This adds new language features to acorn. Otherwise BigInt and other input would not be parsed correct and would not result in nice error messages when using simple assert. PR-URL: nodejs#27400 Refs: nodejs#27391 Refs: nodejs#25835 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR
added a commit
to BridgeAR/node
that referenced
this pull request
Apr 30, 2019
This adds stage-3 language features to acorn so that it's possible to parse these features when using top level await in the REPL. PR-URL: nodejs#27400 Refs: nodejs#27391 Refs: nodejs#25835 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR
added a commit
to BridgeAR/node
that referenced
this pull request
Apr 30, 2019
This adds stage-3 language features to acorn so that the REPL is able to parse these features properly. Otherwise these would cause SyntaxErrors. PR-URL: nodejs#27400 Fixes: nodejs#27391 Fixes: nodejs#25835 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR
added a commit
to BridgeAR/node
that referenced
this pull request
Apr 30, 2019
PR-URL: nodejs#27400 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
|
Thanks for the reviews. Landed in 98e9de7...3d2409c 🎉 |
targos
pushed a commit
that referenced
this pull request
Apr 30, 2019
This adds bigint, class-fields, numeric-separators, static-class features, private class methods and fields as dependency. That way it's possible to use these in combination with acorn to parse these language features. This also removes a couple of files that were not necessary for Node.js to reduce the code base. PR-URL: #27400 Refs: #27391 Refs: #25835 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos
pushed a commit
that referenced
this pull request
Apr 30, 2019
This adds new language features to acorn. Otherwise BigInt and other input would not be parsed correct and would not result in nice error messages when using simple assert. PR-URL: #27400 Refs: #27391 Refs: #25835 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos
pushed a commit
that referenced
this pull request
Apr 30, 2019
PR-URL: #27400 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Merged
targos
added a commit
that referenced
this pull request
May 7, 2019
Notable changes:
* deps:
* Updated llhttp to 1.1.3. This fixes a bug that made Node.js' HTTP
parser refuse any request URL that contained the "|" (vertical bar)
character. #27595
* tls:
* Added an `enableTrace()` method to `TLSSocket` and an `enableTrace`
option to `tls.createServer()`. When enabled, TSL packet trace
information is written to `stderr`. This can be used to debug TLS
connection problems. #27497
#27376
* cli:
* Added a `--trace-tls` command-line flag that enables tracing of TLS
connections without the need to modify existing application code.
#27497
* Added a `--cpu-prof-interval` command-line flag. It can be used to
specify the sampling interval for the CPU profiles generated by
`--cpu-prof`. #27535
* module:
* Added the `createRequire()` method. It allows to create a require
function from a file URL object, a file URL string or an absolute
path string. The existing `createRequireFromPath()` method is now
deprecated #27405.
* Throw on `require('./path.mjs')`. This is technically a breaking
change that should have landed with Node.js 12.0.0. It is necessary
to have this to keep the possibility for a future minor version to
load ES Modules with the require function.
#27417
* repl:
* The REPL now supports multi-line statements using `BigInt` literals
as well as public and private class fields and methods.
#27400
* The REPL now supports tab autocompletion of file paths with `fs`
methods. #26648
* meta:
* Added Christian Clauss (https://github.com/cclauss) to
collaborators. #27554
PR-URL: #27578
targos
added a commit
that referenced
this pull request
May 7, 2019
Notable changes:
* deps:
* Updated llhttp to 1.1.3. This fixes a bug that made Node.js' HTTP
parser refuse any request URL that contained the "|" (vertical bar)
character. #27595
* tls:
* Added an `enableTrace()` method to `TLSSocket` and an `enableTrace`
option to `tls.createServer()`. When enabled, TSL packet trace
information is written to `stderr`. This can be used to debug TLS
connection problems. #27497
#27376
* cli:
* Added a `--trace-tls` command-line flag that enables tracing of TLS
connections without the need to modify existing application code.
#27497
* Added a `--cpu-prof-interval` command-line flag. It can be used to
specify the sampling interval for the CPU profiles generated by
`--cpu-prof`. #27535
* module:
* Added the `createRequire()` method. It allows to create a require
function from a file URL object, a file URL string or an absolute
path string. The existing `createRequireFromPath()` method is now
deprecated #27405.
* Throw on `require('./path.mjs')`. This is technically a breaking
change that should have landed with Node.js 12.0.0. It is necessary
to have this to keep the possibility for a future minor version to
load ES Modules with the require function.
#27417
* repl:
* The REPL now supports multi-line statements using `BigInt` literals
as well as public and private class fields and methods.
#27400
* The REPL now supports tab autocompletion of file paths with `fs`
methods. #26648
* meta:
* Added Christian Clauss (https://github.com/cclauss) to
collaborators. #27554
PR-URL: #27578
This was referenced May 7, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a acorn plugins to properly handle stage-3 language features in the
REPLandassert.I stripped the plugins to the minimum and also removed a couple of files that we will not use to reduce our code overhead in core.
I had to change the require calls in the plugins to work properly with core but I guess that's expected.
The
numeric separatorsare not yet supported byV8but it should be supported by v7.5, so adding it now makes this feature available from the moment we include the new version.There are a few more plugins for other language features:
acorn-dynamic-import,acorn-import-metaandacorn-export-ns-from. These are all @nodejs/modules related and I am not sure if we should include these as well or not. I felt these are not suitable for us but I might be wrong?We currently include the main
acornlicense file which statesCopyright (C) 2012-2018 by various contributors (see AUTHORS). The plugin licenses are alsoMITlicenses but it statesCopyright (C) 2017-2018 by Adrian Heine. I am not sure if thevarious contributorswould be sufficient here, since this is a single person for these modules even though they are hosted in theacornjsorganization.If we have to pull in the other licenses, should I move the plugins out into another folder so that we use a single license for all plugins together (they are all written by the same person / have the same license) or should we include all licenses individually?
Fixes: #27391
Fixes: #25835
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes