GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — src: add support for top level await by devsnek · Pull Request #30370 · nodejs/node

Conversation

@devsnek

@devsnek devsnek commented Nov 12, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Blockers
  • v8:9344
  • v8:9970
  • v8:9978

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
lpinca, lin7sh, bricss, YurySolovyov, shrujalshah28, BridgeAR, SimenB, aidant, LinusU, christianbundy, and 13 more reacted with thumbs up emoji lin7sh, bricss, GaryGSC, YurySolovyov, aidant, LinusU, DaemonBeast, konojunya, codebytere, gokulkrishh, and 4 more reacted with hooray emoji lin7sh, bricss, GaryGSC, YurySolovyov, ashfahan, aidant, LinusU, and DaemonBeast reacted with heart emoji lin7sh, bricss, aidant, LinusU, DaemonBeast, and dnalborczyk reacted with rocket emoji
All reactions
  • 👍 23 reactions
  • 🎉 14 reactions
  • ❤️ 8 reactions
  • 🚀 6 reactions
@devsnek devsnek added vm Issues and PRs related to the vm subsystem. experimental Issues and PRs related to experimental features. esm Issues and PRs related to the ECMAScript Modules implementation. labels Nov 12, 2019
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 12, 2019
@devsnek devsnek removed the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 12, 2019
@devsnek

devsnek commented Nov 12, 2019

Copy link
Copy Markdown
Member Author

cc @nodejs/modules-active-members

TLA modules always evaluate to undefined, so this breaks node -pe for modules
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
devsnek
devsnek commented Nov 12, 2019
View reviewed changes
Comment thread src/module_wrap.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.
@devsnek

This comment has been minimized.

Sign in to view
@lin7sh

This comment has been minimized.

Sign in to view
@devsnek

This comment has been minimized.

Sign in to view
@devsnek
devsnek force-pushed the feature-tla branch 3 times, most recently from 7479161 to 3bb22b2 Compare November 12, 2019 09:07
@bnoordhuis

This comment has been minimized.

Sign in to view
@devsnek

This comment has been minimized.

Sign in to view
@bnoordhuis

This comment has been minimized.

Sign in to view
@devsnek

This comment has been minimized.

Sign in to view
@hybrist

This comment has been minimized.

Sign in to view
@devsnek

This comment has been minimized.

Sign in to view
@devsnek

devsnek commented Nov 12, 2019

Copy link
Copy Markdown
Member Author

opened v8:9968 to address the cache issue.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@devsnek devsnek added the blocked PRs that are blocked by other issues or PRs. label Nov 12, 2019
@devsnek
devsnek force-pushed the feature-tla branch from 3bb22b2 to bb32ff1 Compare November 12, 2019 21:26
@guybedford

guybedford commented Nov 12, 2019

Copy link
Copy Markdown
Contributor

Great work. Are there any roadblocks to asyncifying node -pe lifecycle? Those are the only failing tests?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@devsnek
devsnek force-pushed the feature-tla branch from bb32ff1 to 2ab8402 Compare November 12, 2019 21:36
@devsnek

devsnek commented Nov 12, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

@guybedford it evaluates fine. the difference is that TLA modules always evaluate to undefined, so -p is kind of useless.

I'm not sure whether -p for modules should throw or just be undefined. Thoughts?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@guybedford

guybedford commented Nov 12, 2019

Copy link
Copy Markdown
Contributor

Surely the promise can still return the last expression output though? Perhaps a v8 feature?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@guybedford

guybedford commented Nov 12, 2019

Copy link
Copy Markdown
Contributor

I guess -p for modules makes no sense actually... Let's just make sure we properly throw for this case always.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@weswigham

This comment has been minimized.

Sign in to view
@devsnek

This comment has been minimized.

Sign in to view
@devsnek
devsnek force-pushed the feature-tla branch from 2ab8402 to 9ce585e Compare November 13, 2019 02:55
@devsnek
devsnek force-pushed the feature-tla branch from 20f1d8a to a93964f Compare May 12, 2020 06:26
@nodejs-github-bot

nodejs-github-bot commented May 12, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31291/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins

MylesBorins commented May 12, 2020

Copy link
Copy Markdown
Contributor

If there are no objections I would like to land this in 48 hours
RomainLanz, benjamingr, and GeoffreyBooth reacted with thumbs up emoji LinusU reacted with hooray emoji
All reactions
  • 👍 3 reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@devsnek

devsnek commented May 12, 2020

Copy link
Copy Markdown
Member Author

@MylesBorins i'd like addaleax to review the changes i made to the options parser first.
MylesBorins reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
frank-dspeed
frank-dspeed approved these changes May 13, 2020
View reviewed changes

@frank-dspeed frank-dspeed left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/module_wrap.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 13, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31311/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 14, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31316/
MylesBorins reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@devsnek
src: add support for TLA
5ae5262 
PR-URL: nodejs#30370
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@devsnek
devsnek force-pushed the feature-tla branch from a93964f to 5ae5262 Compare May 14, 2020 17:39
@devsnek

devsnek commented May 14, 2020

Copy link
Copy Markdown
Member Author

Landed in 5ae5262
lin7sh and frank-dspeed reacted with thumbs up emoji hybrist, BridgeAR, aidant, lin7sh, karlhorky, tomasgvivo, frank-dspeed, and banou26 reacted with hooray emoji lin7sh and frank-dspeed reacted with heart emoji guybedford, BridgeAR, aidant, lin7sh, legendecas, and frank-dspeed reacted with rocket emoji
All reactions
  • 👍 2 reactions
  • 🎉 8 reactions
  • ❤️ 2 reactions
  • 🚀 6 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@devsnek
devsnek merged commit 5ae5262 into nodejs:master May 14, 2020
@devsnek
devsnek deleted the feature-tla branch May 14, 2020 17:41
@devsnek devsnek added the notable-change PRs with changes that should be highlighted in changelogs. label May 14, 2020
MylesBorins pushed a commit that referenced this pull request May 14, 2020
@devsnek @MylesBorins
src: add support for TLA
0d77eec 
PR-URL: #30370
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
codebytere added a commit that referenced this pull request May 18, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
2324d42 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) remove obsolete completer variable (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) improve repl autocompletion for require calls (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) replace hard coded core module list with actual list (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370
test:
  * (SEMVER-MINOR) refactor test/parallel/test-bootstrap-modules.js (Ruben Bridgewater) #33282

PR-URL: TODO
@codebytere codebytere mentioned this pull request May 18, 2020
v14.3.0 proposal #33452
Merged
codebytere added a commit that referenced this pull request May 19, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
76a52dd 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370

PR-URL: TODO
codebytere added a commit that referenced this pull request May 19, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
a33e87c 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370

PR-URL: TODO
codebytere added a commit that referenced this pull request May 19, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
5d81e4d 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370

PR-URL: TODO
codebytere added a commit that referenced this pull request May 19, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
c841f51 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370

PR-URL: #33452
@codebytere

codebytere commented Jun 9, 2020

Copy link
Copy Markdown
Member

@devsnek can this go back to v12.x? I'm not sure if the version of v8 on that branch is compatible, so i figured i'd ask before trying
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@devsnek

devsnek commented Jun 9, 2020

Copy link
Copy Markdown
Member Author

@codebytere i don't think anything here is compatible.
MylesBorins and aduh95 reacted with eyes emoji
All reactions
  • 👀 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@codebytere codebytere added the dont-land-on-v12.x label Jun 9, 2020
@frank-dspeed frank-dspeed mentioned this pull request Jul 24, 2020
Support optional chaining operator lebab/lebab#311
Closed
@aduh95 aduh95 mentioned this pull request Oct 30, 2021
process: refactor execution #40664
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GeoffreyBooth GeoffreyBooth GeoffreyBooth left review comments
@addaleax addaleax addaleax left review comments
@benjamingr benjamingr benjamingr approved these changes
@guybedford guybedford guybedford approved these changes
+3 more reviewers
@MylesBorins MylesBorins MylesBorins approved these changes
@targos targos targos left review comments
@frank-dspeed frank-dspeed frank-dspeed approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. experimental Issues and PRs related to experimental features. notable-change PRs with changes that should be highlighted in changelogs. vm Issues and PRs related to the vm subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

18 participants

@devsnek @lin7sh @bnoordhuis @hybrist @guybedford @weswigham @littledan @addaleax @BridgeAR @nodejs-github-bot @lundibundi @frank-dspeed @tomasgvivo @MylesBorins @GeoffreyBooth @benjamingr @codebytere @targos
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.