Skip to content

stream: export consumers and web in parent builtin#40197

Open
Mesteery wants to merge 2 commits into
nodejs:mainfrom
Mesteery:repl-global-sub-builtins
Open

stream: export consumers and web in parent builtin#40197
Mesteery wants to merge 2 commits into
nodejs:mainfrom
Mesteery:repl-global-sub-builtins

Conversation

@Mesteery

@Mesteery Mesteery commented Sep 23, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

No description provided.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels Sep 23, 2021
@Mesteery Mesteery changed the title repl: add subpaths builtins to auto-loadable modules module: add subpaths builtins in addBuiltinLibsToObject Sep 24, 2021
@Mesteery Mesteery added module Issues and PRs related to the module subsystem. doc Issues and PRs related to the documentations. labels Sep 24, 2021
@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from f37c612 to 10f6fd1 Compare September 25, 2021 13:52
@Mesteery Mesteery changed the title module: add subpaths builtins in addBuiltinLibsToObject module,repl: add subpaths builtins in addBuiltinLibsToObject Sep 25, 2021
@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from 10f6fd1 to beedc80 Compare September 25, 2021 14:01
@Mesteery

Mesteery commented Sep 25, 2021

Copy link
Copy Markdown
Contributor Author

cc @nodejs/repl

jasnell
jasnell reviewed Sep 25, 2021
View reviewed changes
Comment thread doc/api/repl.md Outdated
@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from c6bf3e1 to 080ecdd Compare September 25, 2021 17:16
@Mesteery Mesteery changed the title module,repl: add subpaths builtins in addBuiltinLibsToObject module: add subpaths builtins in addBuiltinLibsToObject Sep 25, 2021
@Mesteery Mesteery changed the title module: add subpaths builtins in addBuiltinLibsToObject module: add submodules builtins in addBuiltinLibsToObject Sep 25, 2021
@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from 080ecdd to dfc66e0 Compare September 25, 2021 17:29
@Mesteery Mesteery removed the repl Issues and PRs related to the REPL subsystem. label Sep 25, 2021
@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from e240fbc to d35e7b3 Compare October 9, 2021 13:23
@Mesteery Mesteery changed the title module: add submodules builtins in addBuiltinLibsToObject lib: export submodules in parent builtins Oct 9, 2021
@Mesteery Mesteery added lib / src Issues and PRs related to general changes in the lib or src directory. and removed doc Issues and PRs related to the documentations. module Issues and PRs related to the module subsystem. labels Oct 9, 2021
@Mesteery
Mesteery requested a review from jasnell October 9, 2021 13:25
@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from d35e7b3 to cfff2a3 Compare October 11, 2021 15:32
@Mesteery
Mesteery requested review from addaleax and targos October 11, 2021 15:32
@Mesteery Mesteery mentioned this pull request Oct 14, 2021
Closed
2 tasks
@Mesteery

Mesteery commented Nov 25, 2021

Copy link
Copy Markdown
Contributor Author

Fwiw, that's not what I was suggesting. Just expose consumers as a property of stream.

@jasnell Is this what you are suggesting? By the way I added a test to make sure that all current and future modules export their "submodules" if there are any.

@targos

targos commented Nov 26, 2021

Copy link
Copy Markdown
Member

It seems to break a few tests.

@Mesteery

Mesteery commented Nov 26, 2021

Copy link
Copy Markdown
Contributor Author

It seems to break a few tests.

This is because

node/lib/internal/bootstrap/loaders.js

Line 294 in 43e1278

getOwn(this.exports, exportName, this.exports));
calls the getter and thus triggers the ExperimentalWarning of stream/web.

I don't really know what to do in this case.

@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from cfff2a3 to 7e24afa Compare February 14, 2022 11:03
@Mesteery

Mesteery commented Feb 14, 2022

Copy link
Copy Markdown
Contributor Author

Updated to export only stable submodules in their parent builtin.

@Mesteery Mesteery added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Feb 15, 2022
@aduh95

aduh95 commented May 11, 2024

Copy link
Copy Markdown
Contributor

This needs a rebase.

@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch 2 times, most recently from 6b4f5ed to 48ffa4b Compare May 12, 2024 14:08
@Mesteery
Mesteery force-pushed the repl-global-sub-builtins branch from 48ffa4b to 00c95b4 Compare May 12, 2024 14:09
@Mesteery Mesteery changed the title lib: export submodules in parent builtins stream: export consumers and web in parent builtin May 12, 2024
@Mesteery Mesteery added stream Issues and PRs related to the stream subsystem. and removed lib / src Issues and PRs related to general changes in the lib or src directory. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels May 12, 2024
@aduh95

aduh95 commented May 21, 2024

Copy link
Copy Markdown
Contributor

There are some linter error that needs fixing

@Ethan-Arrowood

Ethan-Arrowood commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Hello, I'm doing some backlog hygiene on all PRs related to networking and streaming modules. There hasn't been any activity here for quite a while so I'm going to mark it stalled. It will auto-close in ~30 days. This is not a rejection; if you've like to pick it back up, rebase onto latest main and give me a ping.

Thank you!

@Ethan-Arrowood Ethan-Arrowood added the stalled Issues and PRs that are stalled. label Jul 15, 2026
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@targos targos Awaiting requested review from targos
@addaleax addaleax Awaiting requested review from addaleax

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. stalled Issues and PRs that are stalled. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Mesteery @targos @aduh95 @Ethan-Arrowood @jasnell @addaleax @nodejs-github-bot