Skip to content
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

Add new items to ToDo list & add links to PRs #79

Merged
merged 2 commits into from May 11, 2022

Conversation

JakobJingleheimer
Copy link
Contributor

No description provided.

@GeoffreyBooth
Copy link
Member

Do we want to distinguish between future items we're planning on doing and others that are tentative? Not sure if we have any tentative ones in this list though.

Losing the headings means we have no way to indicate an “in progress” item.

We should add an item for helper functions to reduce boilerplate. It's implied by the conclusion paragraph but since that was written I think we know we want to do that in some form.

@JakobJingleheimer
Copy link
Contributor Author

JakobJingleheimer commented May 10, 2022

Do we want to distinguish between future items we're planning on doing and others that are tentative? Not sure if we have any tentative ones in this list though.

Sure :) Cross that bridge when we come to it?

Losing the headings means we have no way to indicate an “in progress” item.

It'll have a PR link and not be checked off. Is that too subtle? (We also have a project in the nodejs/node repo)

We should add an item for helper functions to reduce boilerplate. It's implied by the conclusion paragraph but since that was written I think we know we want to do that in some form.

Oh! Yes, good shout. Will add. Done

Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

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

The last paragraph repeats about the thread but this is fine.

@JakobJingleheimer
Copy link
Contributor Author

Oh, actually now that you mention it, didn't Bradley already implement the channel communication thing? I'm thinking we just axe both of those from the final paragraph.

@GeoffreyBooth
Copy link
Member

Oh, actually now that you mention it, didn't Bradley already implement the channel communication thing?

Not that I'm aware of? You can look at landed PRs that he's authored.

@cspotcode
Copy link
Contributor

Pretty sure that a communication channel is already exposed to globalPreload code, if that's what you're talking about.
https://nodejs.org/dist/latest-v18.x/docs/api/esm.html#globalpreload

@JakobJingleheimer
Copy link
Contributor Author

Pretty sure that a communication channel is already exposed to globalPreload code, if that's what you're talking about. https://nodejs.org/dist/latest-v18.x/docs/api/esm.html#globalpreload

Yes, exactly what I was thinking of (here's the PR nodejs/node#39240)

@GeoffreyBooth GeoffreyBooth merged commit c4368c8 into main May 11, 2022
@GeoffreyBooth GeoffreyBooth deleted the add-sync-resolve-and-move-loader-thread branch May 11, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants