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

Unflagging self reference specifiers #403

Closed
jkrems opened this issue Oct 18, 2019 · 29 comments
Closed

Unflagging self reference specifiers #403

jkrems opened this issue Oct 18, 2019 · 29 comments

Comments

@jkrems
Copy link
Contributor

jkrems commented Oct 18, 2019

Background

Open Questions

Use Cases

We know of a handful of use cases:

  • "Test exports": Write unit tests for the public interface of a package using exports.
  • "Copyable examples": Write example code that works both within a checkout of a library's code and while using the library.
  • "Project root require": Easily reference code from other parts of a project's directory structure. Usually expected to not be affected by external visibility.

All of these can be solved today, to a certain degree, using symlinks, special scripts, or features like loaders/require extensions. But there's no first-class support for them in node.

We need to decide which use cases are in scope and which aren't. Specifically: which use cases are in scope for shipping modules themselves. A use case not being in scope doesn't mean we should prevent it.

Outcome
  • Are there other use cases we should track?
  • Which use cases do we need to support for unflagging modules?
  • Which use cases do we want node to encourage?
  • For the use cases we need to support for unflagging, are there alternate solutions?

Shorter Syntactic Sugar

Some people have called for a shorter syntax to be used either in addition or instead of the package name. Most suggestions so far have been ASCII characters but each choice has come with concerns about existing uses:

  • ~: Used in the ecosystem for a different feature (require internal project files from a static root, not relative to the package scope). But that feature is close enough that it could count as ecosystem precedent. May be confused with posix expansion of home directories when an unquoted ~ is used in shells.
  • @: Used in the ecosystem for a different feature (npm scopes). But that feature is close enough that it could count as ecosystem precedent. Also used in some languages (e.g. CoffeeScript, ruby) as a reference to "this" or "self".
  • %: Used in (URL) specifiers already for percentage encoding. Used in Windows for variable substitutions (%HOME%). Sometimes associated with "placeholder".
  • ^: Used in regular expressions and other places as "start" or "beginning". Can also refer to the control key.

Beyond ASCII, we could also use other characters since JS sources are usually UTF8:

  • 📦: Package. Because it's relative to the package.
  • 🦄: A truly special character.
Outcome
  • Do we need a shorter specifier for shipping modules or could it be added later?
  • Should there be a shorter specifier?
  • Which short specifier should it be?

Opting In

This applies mostly to a world with syntactic sugar.

There are two downsides of sugar that may make opt-ins for them necessary:

  • Every one of those characters may have been used as directory names, especially in CJS and on *nix systems. Which means that resolving them to the active package may not be what the author intended to happen.
  • Without an opt-in or parsing every file within the scope, a pre-generated resolution map (e.g. import maps) would have to duplicate every single package's resolution. Once for public use, once for internal use with the special sugar syntax. This doesn't apply to the same degree for the name itself since in many cases those entries could collapse (depending on the exact URL structure).
Outcome
  • Do we need an opt-in for shipping modules or could it be added later?
  • Should we require an opt-in?
  • Should we offer an opt-out?
  • How should the opt-in or opt-out be expressed?

Resolution Order

In short: Should it apply before or after looking at node_modules. The safer choice is likely "after node_modules" but with opt-in it may also be possible to do it before.

Outcome
  • If there is an opt-in, should this feature apply before or after node_modules?
  • If there is no opt-in, should this feature apply before or after node_modules?
  • If there is an opt-out, should this feature apply before or after node_modules?
@ljharb
Copy link
Member

ljharb commented Oct 18, 2019

If there’s an opt-in, then perhaps the user can determine their own sigil? ie:

"magic": true

could enable the package name (and require one to be present in package.json), and:

"magic": "~"

would enable use of the tilde, etc.

Perhaps that compromise would allow everyone to get the sigil they want without needing to bless one, or to be able to use the package name - and being opt-in, it would be able to ignore node_modules entirely.

We could also support a more complex form if people wanted both the name and a sigil (but that seems like too much to me to go in the first unflagged iteration).

@devsnek
Copy link
Member

devsnek commented Oct 18, 2019

at that point, why not "loaders": ["tilde"]

@guybedford
Copy link
Contributor

guybedford commented Oct 18, 2019

Thanks @jkrems for your ongoing work on this feature, it is really really appreciated. I know having a PR open for a long period of time can be incredibly demotivating, but this would provide some really great usability features for packages.

I have concerns about this feature applying after a node_modules check because it theoretically could become unreliable with multi-versions of packages.

For example, say you have pkg@1.0.0 and pkg@2.0.0 in the same app, where one of them is aliased. if we go with the package.json "name" and you could have a folder structure like:

node_modules/pkg [1.0.0]
node_modules/pkg-2 [2.0.0]

inside of node_modules/pkg-2 a require('pkg') would do a node_modules lookup an then return the package instance for pkg@1.0.0 from within pkg@2.0.0.

This seems risky to me in that we no longer have the guaranteed semantics of this feature for package authors, and possible hidden bugs.

If the reason for not supporting this is CommonJS backwards compatibility, perhaps we can do a CIGTM run with the node_modules lookup removed and see how it fares? I suspect every use of require('pkg') in a package at node_modules/pkg will want itself, and that the edge cases will be so rare as to not make this breaking, but this sort of testing can expose that compatibility.

Then based on that feedback we could decide what to do further.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

inside of node_modules/dep/node_modules/pkg a require('pkg') would do a node_modules lookup an then return the package instance for pkg@2.0.0 from within pkg@1.0.0.

I don't think so..? In your example, each pkg@1 and pkg@2 will find themselves if they try to require('pkg'). That seems to match what would happen if the order were reversed.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

Gotcha for my comment: If the nested pkg@2 was installed using an alias as node_modules/dep/node_modules/pkg-aliased, it would find pkg@1. But that feels like a stretch and not like a likely bug people would encounter. I would expect aliases to be used top-level most of the time.

@guybedford
Copy link
Contributor

guybedford commented Oct 18, 2019

I've updated the example to instead use a base-level alias. If we are arguing that this is rare enough not to be an issue, then surely it is rare enough not to be an issue on the backwards compatibility path?

It would just be useful to run the smoke tests and see how this fares to see if we can go this route.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

If we are arguing that this is rare enough not to be an issue, then surely it is rare enough not to be an issue on the backwards compatibility path?

I think it's a difference between new users and existing users. An existing user is allowed to depend on the existing behavior, however rare their use case or solution may be. A new user is expected to accept the restrictions of the new feature, especially if their use case or solution is unusual or rare.

@GeoffreyBooth
Copy link
Member

I assume it probably wasn’t meant to be serious, but oh how I would love it if we actually shipped 📦 as the sigil. Would such a thing be a first in a programming language or runtime?

I don’t think the sigil should be configurable. We just need to make a decision, even if it takes votes. And we can ship the “name” version now and add the sigil later.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

One thing worth noting: I personally think discouraging the use of this feature outside of tests and examples is good™. So I don't believe that breaking advanced package layouts is a showstopper. It depends on what we say about the use cases we aim to support.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

I assume it probably wasn’t meant to be serious, but oh how I would love it if we actually shipped 📦 as the sigil. Would such a thing be a first in a programming language or runtime?

It was serious. I think it's a good way to cut through the very subjective arguments for the various ASCII characters, all of which are terribly overloaded by decades of use in programming languages and computer systems.

@ljharb
Copy link
Member

ljharb commented Oct 18, 2019

@GeoffreyBooth why don't you think the sigil should be configurable?

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

I'm +1 on not making it configurable. It would mean that not only would people have to learn about this kind of resolution, they would have to check for every single directory/project what the syntax is and may get confused every once in a while encountering yet another opinion on the proper sigil.

@GeoffreyBooth
Copy link
Member

@GeoffreyBooth why don’t you think the sigil should be configurable?

Because configurability of this feature adds no value to users. It’s just punting a decision to users that we can make ourselves.

It was serious. I think it’s a good way to cut through the very subjective arguments for the various ASCII characters

For that matter, we could also have 🏠 to mean process.cwd(), for code anywhere to be able to refer back to the root of the app. That’s the other use case for this that I think will be common; for app developers, as opposed to package authors, to be able to refer to the root without needing ../../.. etc. They can do that for 📦 within the package scope of the app, but not from within a package within the app (which may not be desirable regardless).

@ljharb
Copy link
Member

ljharb commented Oct 18, 2019

The most common solution for this already is to use a babel transform, that is already configurable and might vary per project, or even per directory - I'm not sure the confusion is really a big problem in practice.

@GeoffreyBooth that's the problem - we haven't been able to make the decision ourselves.

@GeoffreyBooth
Copy link
Member

If everyone involved agreed on a single sigil, whatever it was, and there was no dispute and the decision was unanimous; then if someone proposed, hey, let’s make this configurable, I think the response would be, “why?” It just doesn’t add value to users, and it adds cost: one more thing to learn, one more thing in the docs, one more thing to maintain and test, one more thing to be aware of by both users and tools, etc. Configurability shouldn’t be a crutch to avoid making decisions.

@devsnek
Copy link
Member

devsnek commented Oct 18, 2019

To be fair, the unconfigurable variant is also one more thing to learn, one more thing in the docs, one more thing to maintain and test, one more thing to be aware of by both users and tools, etc.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

To be fair, the unconfigurable variant is also one more thing to learn, one more thing in the docs, one more thing to maintain and test, one more thing to be aware of by both users and tools, etc.

Right, it's one more thing to learn. But not n more things to learn, one for every individual package under the sun. That's a fundamental difference. :)

@devsnek
Copy link
Member

devsnek commented Oct 18, 2019

Well if you consider node's resolution system the one true resolution system, then sure it's one. But if you include all the other things that can support esm, you're back to n.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

But if you include all the other things that can support esm, you're back to n.

If you adjust one number, then please also write down the other. If we include all the other things and assume that there are n other things (which is a pretty bold assumption, "ecosystem package count n is equal to number of build tools"), we're up to n^2 for the configurable case.

I think realistically the number of tools is closer to a constant when compared to the total number of packages and apps. In which case we're absolutely not up to n but still at 1 for the non-configurable case.

@devsnek
Copy link
Member

devsnek commented Oct 18, 2019

@jkrems i'm arguing that this feature shouldn't exist, not that it should be configurable :)

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

@devsnek Which feature? Or rather: which use case do you think shouldn't be supported?

@devsnek
Copy link
Member

devsnek commented Oct 18, 2019

@jkrems specifically: self-reference specifiers. Generally: anything strengthening the idea of package boundaries.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

@devsnek That's not the question I asked though. I asked which of the use cases you would like or wouldn't like to see supported. The question I asked was specifically aimed at looking at alternatives. Nothing I asked implied that it would be based on package boundaries.

@devsnek
Copy link
Member

devsnek commented Oct 18, 2019

@jkrems i'd like to see all the use cases be addressed somehow. My concerns lie more with the specific solutions currently suggested for certain use cases. I don't always have a better solution on-hand, but sometimes it is better to take more time on a problem than to push out the first solution we come to.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

Do you think any of these use cases need additional support for shipping modules? Or do you think some or all of them could be addressed independently?

@devsnek
Copy link
Member

devsnek commented Oct 18, 2019

I think we've already gone above and beyond (maybe even too far beyond) what is needed for stable modules.

Back on topic: iirc there were some quiet discussions about import maps supporting this case as well. I think it might be valuable for us to engage a bit more with import maps, and maybe also with yarn/npm/etc, as they are in a prime position to generate import maps.

@jkrems
Copy link
Contributor Author

jkrems commented Oct 18, 2019

the first solution we come to.

Side note: This can easily be read as dismissive of other people's work. I hope you are aware that this is not the first solution we came up with. It's the result of many months of iteration, with various solutions being considered by multiple people. I assume that's not what you meant but to me it sounded a bit dismissive of the effort that went into the current proposal.

@guybedford
Copy link
Contributor

It's worth noting we do have a design for a more configurable version of this feature via package imports. See https://github.com/jkrems/proposal-pkg-exports#3-imports-field. That allows a configurable way of setting this specifier as well.

@aduh95
Copy link

aduh95 commented Apr 29, 2020

Fixed by nodejs/node#31002?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants