Node.js Loaders Team Meeting 2023-09-12 #160

Activity

1. mhdawson

   self-assigned this

   on Sep 8, 2023
2. GeoffreyBooth commented on Sep 8, 2023

   

   GeoffreyBooth

   on Sep 8, 2023

   Member

   More actions

   Adding nodejs/node#49432 to the agenda. @LiviaMedeiros, could you join us in this meeting?
3. LiviaMedeiros commented on Sep 9, 2023

   

   LiviaMedeiros

   on Sep 9, 2023

   Member

   More actions

   No; I'll probably watch it next morning. Unless there are any specific questions for the agenda, most of my objections are already outlined in prior discussions anyway.
4. GeoffreyBooth commented on Sep 9, 2023

   

   GeoffreyBooth

   on Sep 9, 2023

   Member

   More actions

   No; I'll probably watch it next morning. Unless there are any specific questions for the agenda, most of my objections are already outlined in prior discussions anyway.

   I'm hoping that we can reach a consensus on what to do regarding that proposal. I'd appreciate your input in that discussion. If we decide something and you don't like our conclusion, what then?

   Is there a better time that would work for you?
5. JakobJingleheimer commented on Sep 12, 2023

   

   JakobJingleheimer

   on Sep 12, 2023

   Member

   More actions

   Are we meeting today?
6. LiviaMedeiros commented on Sep 12, 2023

   

   LiviaMedeiros

   on Sep 12, 2023

   Member

   More actions

   Is there a better time that would work for you?

   Sorry for very late response and/or inconvenience; I can't attend. Please proceed without me at the time that is convenient for others.
7. GeoffreyBooth commented on Sep 12, 2023

   

   GeoffreyBooth

   on Sep 12, 2023

   Member

   More actions

   Are we meeting today?

   Yes

   👍React with 👍1Reacted by Jacob Smith
8. GeoffreyBooth commented on Sep 12, 2023

   

   GeoffreyBooth

   on Sep 12, 2023

   Member

   More actions

   I have a sudden conflict, moving the meeting 30 min later. Anyone planning to join besides me and @JakobJingleheimer ?
9. GeoffreyBooth commented on Sep 12, 2023

   

   GeoffreyBooth

   on Sep 12, 2023

   Member

   More actions

   @JakobJingleheimer and I chatted. I didn’t record it because it was just us and I don’t think there’s much point in making a “team” decision as two people. But basically we both like the proposal in nodejs/node#49432, and want to try to get it implemented and landed before 21.0.0. We think it shouldn’t be split up into many small flags. @LiviaMedeiros is this something you want to collaborate on?

   👀React with 👀1Reacted by Jacob Smith
10. GeoffreyBooth

    closed this as completedon Sep 12, 2023
11. LiviaMedeiros commented on Sep 13, 2023

    

    LiviaMedeiros

    on Sep 13, 2023

    Member

    More actions

    I'm not quite sure yet and can't promise anything.

    First of all, my opinion that nodejs/node#49531 should go separately still stays, until there is convincing reason to not (I think, correct code behaviour outweights any subjective preferences, even if preferences are understandable and reasonable) or TSC resolution.

    As for nodejs/node#49432

    • My objection about URL part is completely lifted if the part is removed from proposal.
      If it's changed into rework of how --import works, I'd like to see if there is consensus on how it should work, consensus on implied breaking changes, and consensus on if the changes would work with that proposal (for example, the latest messages propose to make --interactive behave differently depending on the esm flag, which is questionable).
    • My objection about limitation to --import is lifted because it's no longer a limitation.

    Because the solution for making it non-breaking for ecosystem seems to be "it won't work inside of node_modules", I'm concerned about performance implications. This kind of check is very expensive. We can accept it as trade-off when it's an experimental flag, but if the plan is to make it eventually work by default, I'm afraid that we will end up with significant regression that we can't work around without breaking ecosystem.

    Thus said, I think the main idea of nodejs/node#49432 is feasible but it has too much vagueness right now. Not really a fan of knowingly implementing UB and only then thinking how to fix it; I'd prefer if we had clear detailed image of what we need to achieve and only then start implementing it.
12. GeoffreyBooth commented on Sep 13, 2023

    

    GeoffreyBooth

    on Sep 13, 2023

    Member

    More actions

    First of all, my opinion that nodejs/node#49531 should go separately still stays, until there is convincing reason to not (I think, correct code behaviour outweights any subjective preferences, even if preferences are understandable and reasonable) or TSC resolution.

    As I wrote in nodejs/node#49531 (review), I think the “everything” flag needs to land first, but once that’s landed then I think nodejs/node#49629, the unflagged version of enabling extensionless files in module scope, can probably land. I say “probably” because we should allow at least a few weeks after the first PR landing to see if anyone reports issues before we land the follow-up, especially if the follow-up is unflagged. The reason for this ordering is to manage user expectations; I think the vast majority of users care about extensionless files outside of package scopes, rather than within module scope, so if we land the module scope one first we’re likely to get complaints that we didn’t solve the problem for loose extensionless files.

    • My objection about URL part is completely lifted if the part is removed from proposal.

    @aduh95 raised a few points about this too and I’m now curious to see if a different approach might be preferable (the --import idea). I can either take it out of the proposal or just leave it out of the first PR, and it can either land as a standalone separate thing (if we do --import) or a follow-up additional behavior determined by the big flag, or just drop it. This flag will be experimental for likely the entire lifetime of 21.x, October through April, so we’ll have plenty of time to refine what it does.

    Because the solution for making it non-breaking for ecosystem seems to be “it won’t work inside of node_modules“, I’m concerned about performance implications. This kind of check is very expensive.

    I’m pretty sure we’re already paying the cost for this check because currently both the ESM and CommonJS loaders search for the nearest parent package.json for every .js file. It would do the same for extensionless files, but a) they’re a lot less common than .js files, and b) they shouldn’t be any more expensive to load than .js files.

    Not really a fan of knowingly implementing UB and only then thinking how to fix it; I’d prefer if we had clear detailed image of what we need to achieve and only then start implementing it.

    What do you mean by this?