Support index-modules in exports

[simonihmig]

This is not necessarily something I expect to get merged immediately, more so to start a conversation...

The problem this is addressing is that index-files, which are supported in v1 addons and the default node module resolution, are not working ootb in v2 addons, due to our explicit exports mapping. Say you put a foo/index.js module into /src, then currently the only way to import that is by import foo from 'addon/foo/index', which I think is pretty much unexpected.

This is especially annoying when migrating from v1 addons. For example a common suggestion would be to move your addon's addon-test-support to src/test-support. But again that does not allow you to import from addon/test-support as expected, so makes this a breaking change.

Ideally I think, we would allow alternatives, so that import foo from 'addon/foo' can be satisfied either by src/foo.js or src/foo/index.js.

And actually this seems to be possible, with the changes in this PR. Both webpack (docs)and TypeScript seem to support an array of alternative modules in exports. And TypeScript has supported that for long in typesVersions, which is also what the v1 blueprint does!

The only problem: vite supports array-based alternatives, but does no file-check, just takes whatever matches first AFAIU, see vitejs/vite#4439 (comment). Which is not what we need to support this use case.

The alternative to this "just works" approach would be to force users to explicitly define the exports mapping. But this also does not seem good, because

• not too many folks really know about exports and how they work I think. And if resolving an index-module does not work, you won't get a really helpful error message like saying "tweak your exports".
• you might ship breaking changes accidentally
• if you have many nested files using index files, it become very tedious to explicitly map them, as AFAIK you cannot do that with any globs, so you would have an explicit entry per index-file. Which also can get easily out of sync.

Thoughts?

[NullVoxPopuli]

does this syntax support the array form?

[simonihmig]

Not sure what exactly you mean here? Support for arrays in general is given as backed by the above links. You mean in this particular place, like L109?

[NullVoxPopuli]

yeah, like, if we add /index stuff, it should be both for and without the TS exports

[simonihmig]

Ah right! Not sure why I didn't even think about it. I'll add it, should we agree to actually go forward with this!

[NullVoxPopuli]

I mean, you have my vote!
(not that this is a democracy, since we need to convince everyone the technical merits of a thing lol)

[simonihmig]

But what about vite? My understanding is this will fail under vite. Unless we can convince vite maintainers to change course? (not too likely?)

[NullVoxPopuli]

Why's vite matter here?

The need to declare something about index for ember's conventions helps all packagers

[ef4]

I don't think this is part of Node's spec for package.json exports. I just tried it and Node only attempts to resolve the first thing. I'd rather not diverge from Node.

We already auto-generate the app-js entry in package.json, based on your actual files. It doesn't seem that much worse to generate exports too?

[simonihmig]

Closing this, as we agreed to not do this change in the team meeting. The question if we should make our rollup tooling auto-generate this didn't really come to any definite conclusion. We can discuss pros and cons again...