Skip to content

errors,console: refactor to use ES2020 syntax #42088

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

Closed
wants to merge 415 commits into from

Conversation

xtx1130
Copy link
Contributor

@xtx1130 xtx1130 commented Feb 23, 2022

errors,console: refactor to use ES2020 syntax

Just simplified some codes while I'm reading the newest codes for nodejs. PTAL

@nodejs-github-bot nodejs-github-bot added console Issues and PRs related to the console subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. labels Feb 23, 2022
@xtx1130 xtx1130 changed the title errors & console: refactor to use ES2020 syntax errors,console: refactor to use ES2020 syntax Feb 23, 2022
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

This is good!

The only problem is this makes backporting a bit harder since Node.js v12 is still supported and it doesn't support ??.

Let's see what other collaborators think

@richardlau
Copy link
Member

This is good!

The only problem is this makes backporting a bit harder since Node.js v12 is still supported and it doesn't support ??.

Let's see what other collaborators think

Node.js 12 is going End-of-Life in a few months so I wouldn't worry about it too much.

@BridgeAR
Copy link
Member

Was there any performance penalty using ??=?

@xtx1130
Copy link
Contributor Author

xtx1130 commented Feb 24, 2022

const { version } = require('process')
const max = 1000000000

let test = {}

function ifStatement () {
  if (!test.a) {
    test.a = 'TEST'
  }
}

function es2020Operator () {
  test.a ??= 'TEST'
}

console.log(version)

console.time('ifStatement')
for (let i = 0; i < max; i++) {
  ifStatement()
}
console.timeEnd('ifStatement')

console.time('es2020Operator')
for (let i = 0; i < max; i++) {
  es2020Operator()
}
console.timeEnd('es2020Operator')

output:

v16.13.1
ifStatement: 485.139ms
es2020Operator: 474.884ms

Emmm, just a bit faster than if statement in node v16.

@Mesteery Mesteery added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 24, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 24, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@xtx1130
Copy link
Contributor Author

xtx1130 commented Apr 25, 2022

Hi there, while I'm reading source codes of nodejs, I have seen this pr is not merged. Is this can be merged? Node v12 will End-Of-Life in 4.30


edit: I'm sorry that I forgot to use rebase instead of merge :(

@xtx1130 xtx1130 force-pushed the console-ES2020-syntax branch from 85a0cea to f578723 Compare April 25, 2022 09:41
Flarna and others added 9 commits April 25, 2022 17:43
Promises are never destroyed manually therefore it's not needed to
attach an object to track if destroy hook was called already.

PR-URL: nodejs#42402
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
This commit adds a new 'test' module that exposes an API
for creating JavaScript tests. As the tests execute, TAP
output is written to standard output. This commit only supports
executing individual test files, and does not implement
command line functionality for a full test runner.

PR-URL: nodejs#42325
Refs: nodejs#40954
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
This commit makes it possible to add new core modules that can
only be require()'ed and imported when the 'node:' scheme is
used. The 'test' module is the first such module.

These 'node:'-only modules are not included in the list returned
by module.builtinModules.

PR-URL: nodejs#42325
Refs: nodejs#40954
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Notable changes:

doc:
  * add @ShogunPanda to collaborators (Shogun) nodejs#42362
  * deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) nodejs#42149
http:
  * (SEMVER-MINOR) trace http client by perf_hooks (theanarkh) nodejs#42345
deps:
  * upgrade npm to 8.5.5 (npm team) nodejs#42382
  * update undici to 4.15.1 (Michaël Zasso) nodejs#42246

PR-URL: nodejs#42425
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#42284
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#42416
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Cherry-pick 12c8b4d
Original commit message:
    This commit is a suggestion for adding a rule for NULL usages in the
    code base. This will currently report a number of errors which could be
    ignored using // NOLINT (readability/null_usage)

    PR-URL: nodejs#17373
    Reviewed-By: Jon Moss <[email protected]>
    Reviewed-By: Anna Henningsen <[email protected]>
    Reviewed-By: Timothy Gu <[email protected]>
    Reviewed-By: Colin Ihrig <[email protected]>
    Reviewed-By: Michael Dawson <[email protected]>
    Reviewed-By: Sakthipriyan Vairamani <[email protected]>
    Reviewed-By: Tobias Nießen <[email protected]>

Refs: nodejs@12c8b4d

Cherry-pick fc81e80
Original commit message:

    Update cpplint.py to check for inline headers when the corresponding
    header is already included.

    PR-URL: nodejs#21521
    Reviewed-By: Ben Noordhuis <[email protected]>
    Reviewed-By: James M Snell <[email protected]>

Refs: nodejs@fc81e80

Cherry-pick cbc3dd9
Original commit message:

    src, tools: add check for left leaning pointers

    This commit adds a rule to cpplint to check that pointers in the code
    base lean to the left and not right, and also fixes the violations
    reported.

    PR-URL: nodejs#21010
    Reviewed-By: Ben Noordhuis <[email protected]>
    Reviewed-By: Anna Henningsen <[email protected]>
    Reviewed-By: Ruben Bridgewater <[email protected]>
    Reviewed-By: James M Snell <[email protected]>

Refs: nodejs@cbc3dd9

Cherry-pick 9029981
Original commit message:

    tools: fix cpplint.py header rules

    THIS COMMIT SHOULD GO WITH THE NEXT. IT WILL FIND NEW LINT.

    PR-URL: nodejs#26306
    Reviewed-By: Gireesh Punathil <[email protected]>

Refs: nodejs@9029981

Cherry-pick 0a25ace
Original commit message:

    tools: move cpplint configuration to .cpplint

    PR-URL: nodejs#27098
    Reviewed-By: Joyee Cheung <[email protected]>
    Reviewed-By: Daniel Bevenius <[email protected]>

Refs: nodejs@0a25ace

Cherry-pick afa9a72
Original commit message:

    tools: refloat update link to google styleguide for cpplint

    This commit updates two old links to Google's C++ styleguide which
    currently result in a 404 when accessed.

    PR-URL: nodejs#30876
    Reviewed-By: Michaël Zasso <[email protected]>
    Reviewed-By: David Carlier <[email protected]>
    Reviewed-By: Colin Ihrig <[email protected]>
    Reviewed-By: Richard Lau <[email protected]>
    Reviewed-By: Rich Trott <[email protected]>

Refs: nodejs@afa9a72

Cherry-pick e23bf8f
Original commit message:

    tools,src: refloat forbid usage of v8::Persistent

    `v8::Persistent` comes with the surprising catch that it requires
    manual cleanup. `v8::Global` doesn’t, making it easier to use,
    and additionally provides move semantics. New code should always
    use `v8::Global`.

    PR-URL: nodejs#31018
    Reviewed-By: Colin Ihrig <[email protected]>
    Reviewed-By: Richard Lau <[email protected]>
    Reviewed-By: James M Snell <[email protected]>
    Reviewed-By: David Carlier <[email protected]>
    Reviewed-By: Rich Trott <[email protected]>
    Reviewed-By: Gus Caplan <[email protected]>
    Reviewed-By: Joyee Cheung <[email protected]>
    Reviewed-By: Ben Noordhuis <[email protected]>
    Reviewed-By: Stephen Belanger <[email protected]>

PR-URL: nodejs#35569
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>

PR-URL: nodejs#35719
Reviewed-By: Antoine du Hamel <[email protected]>

PR-URL: nodejs#35866

PR-URL: nodejs#36213
Reviewed-By: Franziska Hinkelmann <[email protected]>

PR-URL: nodejs#36235
Reviewed-By: Luigi Pinca <[email protected]>

PR-URL: nodejs#36324
Reviewed-By: Beth Griggs <[email protected]>

PR-URL: nodejs#38851
Reviewed-By: Khaidi Chu <[email protected]>

PR-URL: nodejs#42416
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
https://bugs.chromium.org/p/v8/issues/detail?id=10704 is already
fixed, but since AbortController currently throws ERR_INVALID_THIS
we'll revert to class fields in a subsequent patch. For now just
update the comments.

PR-URL: nodejs#42361
Refs: nodejs@b1c3909
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
targos and others added 27 commits April 25, 2022 17:43
`handler-outside-simulator.cc` uses inline assembly, which is not
supported by MSVC.

PR-URL: nodejs#40488
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
We are not ready to migrate yet.

Refs: nodejs/node-v8#214

PR-URL: nodejs#40907
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=12661
Refs: nodejs#42375

PR-URL: nodejs#42657
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Remove call to `memfd_create`.
The function that references it is only used for V8 testing.

PR-URL: nodejs#42657
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Create an unused deopt kind to replace `DeoptimizeKind::kSoft`, which
was removed. This ensures that the layout of IsolateData doesn't change.

Refs: v8/v8@1ff685d

PR-URL: nodejs#42740
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#42740
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
V8's test runner now requires Python 3. Use the Python binary we
detected that is used elsewhere in the Makefile.

PR-URL: nodejs#42740
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This reverts commit 35d72bf.

PR-URL: nodejs#42740
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
PR-URL: nodejs#42786
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#42803
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
PR-URL: nodejs#42783
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Refs: nodejs#41431
Fixes: nodejs#42787

PR-URL: nodejs#42789
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Add maintaining-webassembly.md with strategy based
on discussion in Next-10 mini-summit:
nodejs/next-10#127

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#42660
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Fixes: nodejs#42703

PR-URL: nodejs#42824
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
PR-URL: nodejs#42797
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
Refs: nodejs#41749
Fixes: nodejs#21130

PR-URL: nodejs#42701
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
export total_global_handles_size, used_global_handles_size,
external_memory in getHeapStatistics

PR-URL: nodejs#42784
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#42815
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
The path of the Python executable might contain white spaces. Handle
this when the `check-python` function is called.

Fixes: nodejs#42801

PR-URL: nodejs#42810
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
fixes: nodejs#42792

PR-URL: nodejs#42807
Fixes: nodejs#42792
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Akhil Marsonya <[email protected]>
It is what V8's build config does by default.

PR-URL: nodejs#42809
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
PR-URL: nodejs#42820
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Refs: nodejs#41263

PR-URL: nodejs#42846
Fixes: nodejs#42741
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: nodejs#42848
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#42845
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
As reported by coverity the null check is
done after earlier code has already derefenced
options.envp. Either we should null check earlier
uses or this check is not necessary. Since options.envp
is created with new which should throw an
exception intead of returning null, it should
be safe to assume the null check is not required.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#42819
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@xtx1130
Copy link
Contributor Author

xtx1130 commented Apr 25, 2022

emm, I am sorry for that, there is something I do wrong, I will close this pr and open a new pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.