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

Node.js Loaders Team Meeting 2022-09-13 #111

Closed
mhdawson opened this issue Sep 9, 2022 · 7 comments
Closed

Node.js Loaders Team Meeting 2022-09-13 #111

mhdawson opened this issue Sep 9, 2022 · 7 comments
Assignees

Comments

@mhdawson
Copy link
Member

mhdawson commented Sep 9, 2022

Time

UTC Tue 13-Sep-2022 18:00 (06:00 PM):

Timezone Date/Time
US / Pacific Tue 13-Sep-2022 11:00 (11:00 AM)
US / Mountain Tue 13-Sep-2022 12:00 (12:00 PM)
US / Central Tue 13-Sep-2022 13:00 (01:00 PM)
US / Eastern Tue 13-Sep-2022 14:00 (02:00 PM)
EU / Western Tue 13-Sep-2022 19:00 (07:00 PM)
EU / Central Tue 13-Sep-2022 20:00 (08:00 PM)
EU / Eastern Tue 13-Sep-2022 21:00 (09:00 PM)
Moscow Tue 13-Sep-2022 21:00 (09:00 PM)
Chennai Tue 13-Sep-2022 23:30 (11:30 PM)
Hangzhou Wed 14-Sep-2022 02:00 (02:00 AM)
Tokyo Wed 14-Sep-2022 03:00 (03:00 AM)
Sydney Wed 14-Sep-2022 04:00 (04:00 AM)

Or in your local time:

Links

Agenda

Extracted from loaders-agenda labelled issues and pull requests from the nodejs org prior to the meeting.

Invited

  • Loaders team: @nodejs/loaders

Observers/Guests

Notes

The agenda comes from issues labelled with loaders-agenda across all of the repositories in the nodejs org. Please label any additional issues that should be on the agenda before the meeting starts.

Joining the meeting

@mhdawson mhdawson self-assigned this Sep 9, 2022
@GeoffreyBooth
Copy link
Member

Anything to meet about?

@JakobJingleheimer
Copy link
Contributor

I think no? I was swamped last week but am back to off-thread this week (and then away 21 Sep – 03 Oct). Guy's wizardry addressed the 2 big issues with which I was contending; now there are a couple smaller ones (like how to get the worker to gracefully terminate when finished so the node process can itself gracefully terminate). Once addressed, I think we can know about ambients.

How come those helper utils? I think node v19.0.0 would be ideal for removing --experimental-specifier-resolution=node which is currently scheduled for a 18 Oct release. Any chance of that happening?

@GeoffreyBooth
Copy link
Member

How come those helper utils? I think node v19.0.0 would be ideal for removing --experimental-specifier-resolution=node which is currently scheduled for a 18 Oct release. Any chance of that happening?

I haven't had time to work on those, and I would appreciate the help. I guess the first step is to identify which ones are really must-have helpers (if any) for that to happen. The CommonJS algorithm is well and baked at the point and lots of third-party libraries reimplement it, so I'm not sure how many helpers really need to be part of Node core. @arcanis?

I guess ideally we would have an example loader that implements the legacy resolution today, with no helpers, and passes all the tests that --experimental-specifier-resolution=node does. Then we could look at that and see what parts of it would be improved via helpers that would have general utility. But maybe all we need is to make the loader, publish it in https://github.com/nodejs/loaders-test, and stop. That should be enough for removal of the flag, and optimizing the loader via new helpers could happen whenever.

@arcanis
Copy link
Contributor

arcanis commented Sep 12, 2022

The CommonJS algorithm is well and baked at the point and lots of third-party libraries reimplement it, so I'm not sure how many helpers really need to be part of Node core. @arcanis?

The exports and imports resolutions are two important ones; people in userland have tried to reimplement them, but those implementations tend to be subject to bugs - when they even exist.

But generally, I feel like the whole CJS resolution algorithm should be exposed as public helpers, as it would avoid the problems we have with userland resolution being out of sync with the standard one (like when the node: protocol was implemented, or even when exports was introduced) - unless there's a good reason why it shouldn't be essentially turned into a standard library?

@GeoffreyBooth
Copy link
Member

The exports and imports resolutions are two important ones

How would this be different from require.resolve? Couldn't we achieve it via createRequire and require.resolve with the paths option set to the pathified import.meta.url? https://nodejs.org/api/modules.html#requireresolverequest-options

Canceling the meeting as we have no agenda.

@arcanis
Copy link
Contributor

arcanis commented Sep 19, 2022

How would this be different from require.resolve? Couldn't we achieve it via createRequire and require.resolve with the paths option set to the pathified import.meta.url? https://nodejs.org/api/modules.html#requireresolverequest-options

It wouldn't work:

  • require.resolve follows the CJS resolution rules, so it has automatic extension resolution. It's unlikely we can change that, since it'd be a fairly major breaking change.
  • If we we're to call require.resolve('/path/to/package/lodash'), it would bypass the exports resolution (because it's an absolute path, and absolute paths don't go through it. If we were to call require.resolve('lodash'), then we wouldn't be able to first tell Node where to find the package (ie /path/to/package/lodash), so it wouldn't work either.

But really, my main point is the second paragraph of my previous post: I don't see reasons why such an important piece of Node is kept internal. Are there reasons why the Node resolution can't be a proper library, accepting a couple of I/O primitives as parameter (ie a host)?

@arcanis
Copy link
Contributor

arcanis commented Sep 19, 2022

See an example of how we have to literally copy/paste the Node implementation when trying to support imports:

https://github.com/yarnpkg/berry/pull/4862/files#diff-17f1ad8d28499b140c2b5a0c48234cfa41296084d66daf7f68641f83d2344d50

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

No branches or pull requests

4 participants