Skip to content

deps: update to cjs-module-lexer@1.1.0 - #37712

Closed
guybedford wants to merge 2 commits into
nodejs:masterfrom
guybedford:cjs-module-lexer-1.1.0
Closed

deps: update to cjs-module-lexer@1.1.0#37712
guybedford wants to merge 2 commits into
nodejs:masterfrom
guybedford:cjs-module-lexer-1.1.0

Conversation

@guybedford

@guybedfordguybedford commented Mar 11, 2021

Copy link
Copy Markdown
Contributor

This update includes the following new features:

This update is fully backwards compatible with 1.0.0.

Ideally we should aim to backport this change as much as possible to maximise compatibility. Hopefully to Node.js 14 at least, not sure if Node.js 12 will be possible at this point.

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Mar 11, 2021
@guybedford

guybedford commented Mar 11, 2021

Copy link
Copy Markdown
ContributorAuthor

@nodejs/modules

@richardlaurichardlau added lts-watch-v12.x semver-minor PRs that contain new features and should be released in the next minor version. labels Mar 11, 2021
@richardlau

richardlau commented Mar 11, 2021

Copy link
Copy Markdown
Member

Wasn't there a reference the the cjs-module-lexer version in the docs somewhere that needs to be updated?

@guybedford

guybedford commented Mar 11, 2021

Copy link
Copy Markdown
ContributorAuthor

@richardlau yes there is, thanks for remembering, added.

@aduh95

aduh95 commented Mar 11, 2021

Copy link
Copy Markdown
Contributor

Should we have a update-cjs-module-lexer.sh script that does that to make sure we don't forget the doc link next time?

@guybedford

guybedford commented Mar 11, 2021

Copy link
Copy Markdown
ContributorAuthor

@aduh95 I'm open to any tooling here, although in theory this project shouldn't be updated too often due to the risk of compatibility divergence between Node.js versions.

MylesBorins
MylesBorins approved these changes Mar 11, 2021
View reviewed changes

@MylesBorinsMylesBorins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

RSLGTM

GeoffreyBooth
GeoffreyBooth approved these changes Mar 11, 2021
View reviewed changes

@GeoffreyBoothGeoffreyBooth left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

RSLGTM

@nodejs-github-bot

nodejs-github-bot commented Mar 11, 2021

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/36637/

Trott
Trott approved these changes Mar 12, 2021
View reviewed changes

@TrottTrott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rubber-stamp LGTM

@MylesBorins

MylesBorins commented Mar 12, 2021

Copy link
Copy Markdown
Contributor

As v12.x is now in maintenance I think the odds of doing a Semver-Minor is quite low. Would you consider this Semver-Minor from a Node.js perspective (adding new features) or Semver-Patch (Fixing broken experience)?

@bmeck

bmeck commented Mar 12, 2021•
edited
Loading

Copy link
Copy Markdown
Member

@MylesBorins this is broken experience if you read the changes to the lexer. It is about some babel output etc. not correctly being processed.

@MylesBorins

MylesBorins commented Mar 12, 2021

Copy link
Copy Markdown
Contributor

@bmeck with that in mind I think while it was Semver-Minor for the lexer the update should be considered Semver-Patch for Node.js and we should backport and include in the next 12.x maintenance release.

Thoughts folks?

@bmeck

bmeck commented Mar 12, 2021

Copy link
Copy Markdown
Member

I think especially given #37718 it should be back ported if it isn't a feature addition. Having 14 and 12 go out of sync over this would be very strange to me.

@richardlau

richardlau commented Mar 12, 2021

Copy link
Copy Markdown
Member

As v12.x is now in maintenance I think the odds of doing a Semver-Minor is quite low. Would you consider this Semver-Minor from a Node.js perspective (adding new features) or Semver-Patch (Fixing broken experience)?

We want to put out #37603 in a 12.x release and that's semver-minor. There are also some (3 as of now) open semver-minor v12.x backports: https://github.com/nodejs/node/pulls?q=is%3Apr+is%3Aopen+label%3Av12.x+label%3Asemver-minor
So we are going to do at least one more semver-minor for 12.x, regardless of the semverness of this PR.

guybedford added a commit that referenced this pull request Mar 17, 2021
@guybedford
deps: update to cjs-module-lexer@1.1.0
10a8dc0
PR-URL: #37712
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@guybedford

guybedford commented Mar 17, 2021

Copy link
Copy Markdown
ContributorAuthor

Landed in 10a8dc0.

@bricssbricss mentioned this pull request Mar 17, 2021
Merged
danielleadams pushed a commit that referenced this pull request Mar 17, 2021
@guybedford@danielleadams
deps: update to cjs-module-lexer@1.1.0
dfca2fa
PR-URL: #37712
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams added a commit that referenced this pull request Mar 17, 2021
@danielleadams
2021-03-17, Version 15.12.0 (Current)
5011009
PR-URL: #37766
Notable changes:
* crypto:
* add optional callback to crypto.sign and crypto.verify (Filip Skokan) #37500
* support JWK objects in create\*Key (Filip Skokan) #37254
* deps:
* update to cjs-module-lexer@1.1.0 (Guy Bedford) #37712
* switch openssl to quictls/openssl (James M Snell) #37601
* fs:
* improve fsPromises writeFile performance (Nitzan Uziely) #37610
* improve fsPromises readFile performance (Nitzan Uziely) #37608
* lib:
* implement AbortSignal.abort() (James M Snell) #37693
* node-api:
* define version 8 (Gabriel Schulhof) #37652
* worker:
* add setEnvironmentData/getEnvironmentData (James M Snell) #37486
danielleadams added a commit that referenced this pull request Mar 17, 2021
@danielleadams
2021-03-17, Version 15.12.0 (Current)
3b3b02f
PR-URL: #37766
Notable changes:
* crypto:
* add optional callback to crypto.sign and crypto.verify (Filip Skokan) #37500
* support JWK objects in create\*Key (Filip Skokan) #37254
* deps:
* update to cjs-module-lexer@1.1.0 (Guy Bedford) #37712
* switch openssl to quictls/openssl (James M Snell) #37601
* fs:
* improve fsPromises writeFile performance (Nitzan Uziely) #37610
* improve fsPromises readFile performance (Nitzan Uziely) #37608
* lib:
* implement AbortSignal.abort() (James M Snell) #37693
* node-api:
* define version 8 (Gabriel Schulhof) #37652
* worker:
* add setEnvironmentData/getEnvironmentData (James M Snell) #37486
richardlau pushed a commit that referenced this pull request Mar 17, 2021
@guybedford@richardlau
deps: update to cjs-module-lexer@1.1.0
1872625
PR-URL: #37712
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@richardlaurichardlau mentioned this pull request Mar 18, 2021
Merged
@RaisinTenRaisinTen mentioned this pull request Mar 26, 2021
Merged
@danielleadams

danielleadams commented Apr 27, 2021

Copy link
Copy Markdown
Contributor

Removing lts-watch-v14.x since 1.1.1 has already been backported here: #38002

@aduh95aduh95 mentioned this pull request Apr 28, 2021
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GeoffreyBoothGeoffreyBoothGeoffreyBooth approved these changes
@TrottTrottTrott approved these changes
@targostargostargos approved these changes
@cjihrigcjihrigcjihrig approved these changes
@aduh95aduh95aduh95 approved these changes
+2 more reviewers
@bmeckbmeckbmeck approved these changes
@MylesBorinsMylesBorinsMylesBorins approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-ciPRs that need a full CI run.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

11 participants

@guybedford@richardlau@aduh95@nodejs-github-bot@MylesBorins@bmeck@danielleadams@GeoffreyBooth@Trott@targos@cjihrig