Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Stabilizing the resolver #451

Closed
guybedford opened this issue Dec 3, 2019 · 7 comments
Closed

Stabilizing the resolver #451

guybedford opened this issue Dec 3, 2019 · 7 comments
Labels
modules-agenda To be discussed in a meeting

Comments

@guybedford
Copy link
Contributor

I'd like to suggest that we mark a new phase target for resolver stability.

In particular the features here are package self-resolution and dual mode support (currently via conditional exports), which we agreed to support by end of Jan in some form.

As per #450, for 12.x stability I think it is important that our changes to the CommonJS resolver can be included in 12.x as soon as possible, since the next release deadline would be for Jan.

The concern is that users shipping "exports" for 13.x users may find the code uses "main" fine in 12.x, but then when we backport modules to 12.x in April the "exports" resolution kicks in on that branch and there may be breakage. To avoid this scenario we would ideally be able to ship "exports" from Jan already now on 12.x so that users shipping "exports" code for 13.x immediately can see the 12.x compatibility of their code, making it a compatibility issue of the act of shipping itself. The sweet spot we are in is that no one has shipped "exports" yet, so we can still define what breakage means. But once enough time passes we no longer have that luxury.

@ljharb
Copy link
Member

ljharb commented Dec 3, 2019

As one data point, I'm planning to release deep-equal v2 and tape v5 within the next few weeks, which will have transitive deps using "exports"; the window for that luxury seems quite short.

@guybedford
Copy link
Contributor Author

Having those packages use the "exports" entry point on an upgrade path for v12 likely won't be breaking though right? The issue is when enough users ship "exports" that the 5% probability of a break is big enough to be an issue.

@ljharb
Copy link
Member

ljharb commented Dec 3, 2019

So, since I was able to use the array RHS form for ., it works on v13.0/v13.1, as well as v13.2. If either's functionality arrives in 12.x, then while it's technically breaking since previously requireable files will stop being so, in practice I doubt it will be an issue.

However, if the exact semantics of either 13.0-1, or 13.2+, aren't what 12.x gets, then the breakage may be significant.

@guybedford
Copy link
Contributor Author

With --experimental-conditional-exports unflagged there are no other current experimental proposals for the proposal, apart from possibly the package imports field - https://github.com/jkrems/proposal-pkg-exports#3-imports-field.

We can leave this issue open to discuss any further proposals or that proposal, or close it. I do think it is important we try to move to a point of considering the resolver as stable as possible... ideally this is not an area that should change too much.

@ljharb
Copy link
Member

ljharb commented Jan 16, 2020

The only remaining item i can think of is a self export sigil.

@DerekNonGeneric
Copy link
Contributor

DerekNonGeneric commented Jan 16, 2020

I was really bummed out that there wasn't enough time to get to this in the previous meeting. I was hoping for some development regarding what @guybedford spoke about in nodejs/node#30514 (comment).

[..] work with core to get the unit tests made as a suite that can be tested against userland resolvers

Am I correct in thinking that this is the issue tracking that effort?

In the meeting referenced in that comment, @MylesBorins spoke about the creation of a repo similar to Test262 that would allow for an embeddable JS implementation of all the ESM resolver pseudocode, which I am just about finished with. The only piece I haven't completed yet is the recent change in nodejs/node#31009 that was just merged to fix the conditional exports bugs (nodejs/node#30602, nodejs/node#30633).

If this is not the correct issue, does it exist?

@guybedford
Copy link
Contributor Author

@DerekNonGeneric thanks for bringing that up. Having portable tests for the resolver is definitely an important part of its stability that has so far been overlooked in this thread (although was discussed in the meeting as you point out).

I've created a new tracking issue for this at nodejs/node#49448.

In terms of new resolver features, personally I'm not likely to push "imports" anytime soon as I feel "exports" gets a good coverage of the usefulness there. But if anyone would like to create issues for this or any other new resolver features please do go ahead.

So I'll close this issue then for now as moving on these sub-issues rather.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
modules-agenda To be discussed in a meeting
Projects
None yet
Development

No branches or pull requests

3 participants