add package exports

[gunters63]

(for Typescript node16 package resolution)

[changeset-bot]

🦋 Changeset detected

Latest commit: 2578152

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@use-gesture/core	Patch
@use-gesture/react	Patch
@use-gesture/vanilla	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2578152:

Sandbox	Source
gesture-drag	Configuration
gesture-drag-target	Configuration
gesture-nested	Configuration
gesture-drag-vanilla	Configuration
gesture-move	Configuration
gesture-pinch	Configuration
gesture-multiple-pinch	Configuration
gesture-three	Configuration
gesture-card-zoom	Configuration
gesture-viewpager	Configuration

[dbismut]

@gunters63 thanks for this, I'm not familiar at all with module resolution to be honest, I'm relying on preconstruct to bundle that for me. There's an export option that maybe we can use to automatize all this? I'll dig deeper into this next week.

PS: in the repro you've posted in #544 @use-gesture/react is missing from deps, but I was able to reproduce the issue after installing it.

[Andarist]

@gunters63 do you have a sample repro case that I could investigate to check out how exactly it's failing today with TS moduleResolution: node16?

[gunters63]

I played a bit with the preconstruct export option and I think it cannot be used for now as it does not generate "types" entries needed by Typescript >= 4.7

[Andarist]

@gunters63 note that types entries are not strictly required. You can rely on "sibling lookup" and it works just fine - this is what preconstruct does at the moment. For each entry in exports you should have a sibling .d.ts file that TS should pick up.

[gunters63]

@Andarist: Does the repo given by @a-type in #544 work?

[gunters63]

@gunters63 note that types entries are not strictly required. You can rely on "sibling lookup" and it works just fine - this is what preconstruct does at the moment. For each entry in exports you should have a sibling .d.ts file that TS should pick up.

Ah ok, so maybe adding the export option to package.json and run preconstruct fix will actually work.

[Andarist]

@gunters63 yes, I've just missed it at first since it wasn't linked to from the description of this PR. Thank you for pointing me in the right direction.

[Andarist]

Ah ok, so maybe adding the export option to package.json and run preconstruct fix will actually work.

I've checked that and ti fixes the problem of looking up the correct type definition file but that file looks, at the moment, like this:

export * from "./declarations/src/index";

and that in turn fails on the lack of the extension here

[gunters63]

@Andarist: Interesting. I thought the extension is only needed for Node. The generated esm and cjs files by preconstruct all have extensions.

[Andarist]

All of those module and default conditions point to the same file - this is wrong, they should point to the files representing those entrypoints. It would be the easiest to generate this through preconstruct - even if you don't end up enabling its exports-related options just yet (u can enable them temporarily, generate this stuff and roll back those options)

[gunters63]

Yes, I just was asking that myself just now. I was of the impression use-gesture still worked in my repo with these patches but I think I didn't look hard enough.

[gunters63]

It works in my repo because I don't use the sub path imports myself in my code :). So, this PR would probably work for me but of course it is wrong.

[Andarist]

@Andarist: Interesting. I thought the extension is only needed for Node. The generated esm and cjs files by preconstruct all have extensions.

Unfortunately, it seems that they are now required in .d.ts too. I made it work locally by adjusting the files in node_modules - the .js extension has to be added to all imports within .d.ts

[gunters63]

I generated the exports again with preconstruct fix (with exports option enabled) and then built successfully with preconstruct build.

When I manually copied the package over to my repos node_modules folder it seemed to work.
Typescript did not choke about the missing extensions in export * from "./declarations/src/index";

I will do more testing tomorrow.

[gunters63]

I updated the PR now to use preconstruct export .

[Andarist]

Ye, so this looks OK-ish now - but we still end up with extensionless imports in the generated .d.ts. That has to be fixed in preconstruct itself.

[gunters63]

@Andarist: Interesting. I thought the extension is only needed for Node. The generated esm and cjs files by preconstruct all have extensions.

Unfortunately, it seems that they are now required in .d.ts too. I made it work locally by adjusting the files in node_modules - the .js extension has to be added to all imports within .d.ts

Where did the missing .js extension in the .d.ts files make problems?

I used the repo from #544 as base, fixed the missing dependency and just added the missing exports to the package,json of core use-gesture manually. Everything was fine after that, typings working, pnpm build also. I then extended the repo with some sample code from use-gestures sandboxes to make sure it was also working at runtime.

[a-type]

I experienced the missing extension causing issues in .d.ts with another package (stitchesjs/stitches#1086). Not sure exactly what conditions cause it, but it was definitely showing up for me there.

[gunters63]

Hmm, if that would be the case (.d.ts files missing file extensions in imports/exports generally cause problems with type resolution under nodenext) then many packages on npm would be broken.

I just checked in our Monorepo (which is all nodenext resolution), most direct dependencies have .d.ts files without extensions.
It would be really interesting to know when exactly this causes a problem.

In any case, the added exports fixed the problem in the reproduction repo (and in our project). I propose to merge this and file a bug report in the preconstruct issue tracker about the extensionless exports/imports.

[gunters63]

In any case, the added exports fixed the problem in the reproduction repo (and in our project). I propose to merge this and file a bug report in the preconstruct issue tracker about the extensionless exports/imports.

Thought a little bit about it more. Adding exports to a npm package can be a breaking change for some consumers if they use sub path imports of files and those are restricted now. Not sure it can be the case here.

[Andarist]

Where did the missing .js extension in the .d.ts files make problems?

I didn't recheck this right now to be 100% confident but I think it starts to be required when you are using moduleResolution: node16 and when the imported package has package.json#exports. This is specifically to match the "modern" node.js requirements (the presence of exports in node.js requires the extension for all relative imports/requires in that package).

I definitely had this problem here, when playing with this locally. I can't be sure how our testing methodologies were different and that we didn't experience the same thing.

Hmm, if that would be the case (.d.ts files missing file extensions in imports/exports generally cause problems with type resolution under nodenext) then many packages on npm would be broken.

That's true - and that's why I believe that the second precondition is required for this. This is only required when a package has package.json#exports (and most packages on npm don't have that, that's why extensionless sources are OK, even with moduleResolution: node16).

In any case, the added exports fixed the problem in the reproduction repo (and in our project). I propose to merge this and file a bug report in the preconstruct issue tracker about the extensionless exports/imports.

I'm against that. I can't be sure without further analysis but I think there is a potential for this PR (in its current form) to break some consumers. I would only proceed with this after we ensure that extensions are there.

Thought a little bit about it more. Adding exports to a npm package can be a breaking change for some consumers if they use sub path imports of files and those are restricted now. Not sure it can be the case here.

That's true. It's up to the maintainer to decide if adding exports to their package is a breaking change or not. I think that it should be mostly fine in this case here - if only done right.

[gunters63]

Where did the missing .js extension in the .d.ts files make problems?

I didn't recheck this right now to be 100% confident but I think it starts to be required when you are using moduleResolution: node16 and when the imported package has package.json#exports. This is specifically to match the "modern" node.js requirements (the presence of exports in node.js requires the extension for all relative imports/requires in that package).

I definitely had this problem here, when playing with this locally. I can't be sure how our testing methodologies were different and that we didn't experience the same thing.

I tried exactly this with the reproduction repo, patched use-gesture's package.json to have those exports and changed the repo's tsconfig.json to moduleResolution node16 and didn't have problems. We should really try to nail this down.

I am not sure the node16 resolution requirement (always have extensions) fully applies to .d.ts files too (which are invisible to Node of course). I didn't find an exact specification in the Typescript documentation.

[Andarist]

Hmm, I've done some new tests after creating a fresh copy of that repro and it seems to work correctly now (even without extensions). I wonder what did I do differently during the weekend - since I definitely experienced a problem like that 🤔

[gunters63]

After thinking a bit more I am pretty sure Typescript does not require exact matches for imports in .d.ts files (with node16 moduleResolution):

The vast majority of all Typescript-supporting npm modules use Typescripts tsc compiler for generating the .d.ts files (either by directly calling tsc or somewhere under the hood). And various methods for creating esm and cjs bundles.

As far as I know tsc never changes paths in imports/exports when emitting files. It does specifically never touch extensions or add missing ones.

File extensions normally don't matter for the ,js files as they are bundled and such have no relative imports.

This means if the .d.ts files really required extensions in imports/exports ALL Typescript files in the npm package sources would have to have extensions too in relative imports/exports even if the package itself does not use moduleResolution node16. This would require significant changes to all packages (to support node16) and would be a big blocker for a quick adaption.

I work with a multi-package monorepo here all-node16 moduleResolution with about 80 direct dependencies and while they where numerous problems with packages not fully supporting node16 (most of them missing exports :)) missing extensions in .d.ts files was never a problem,

[Andarist]

The vast majority of all Typescript-supporting npm modules use Typescripts tsc compiler for generating the .d.ts files (either by directly calling tsc or somewhere under the hood). And various methods for creating esm and cjs bundles.

As far as I know tsc never changes paths in imports/exports when emitting files. It does specifically never touch extensions or add missing ones.

This is true - but note that when compiling libraries with moduleResolution: node16 your imports would have extensions. So in such a case, the extensions would get into .d.ts. And I assume that this would be one of the most popular setups when adding exports to a package.

Of course, .d.ts can also be authored by hand but in such a case I think it would be reasonable for the TS team to require extensions too.

That being said - I agree with you. As long as I can't repro this anymore - I can't say that there is a problem with this. I've also rechecked Emotion (where I added exports a couple of months back), we also don't have extensions there in our .d.ts and I don't recall any complaints about this.

So this is probably good to go. Adding exports is always risky... but it should be OK here, hopefully, nothing will break. We were very cautious about introducing support for exports into Preconstruct - if we've shipped this with success to Emotion (a highly popular library) then I believe that it's a good setup. Note that our exports support deliberately doesn't provide all of the things that people would like it to - that's because satisfying those other requirements can be risky and has some gotchas. So what we've chosen to implement was something that we've tested as being as safe as possible.

[dbismut]

Let's go then! Thanks everyone.

[Andarist]

@dbismut feel free to ping me if anything goes south. I'm happy to help you investigate any potential issues.

[gunters63]

I just tried 10.2.23 in our monorepo and the typing problems went away :)

I am happy to help too if problems arise of course.

[dbismut]

Thanks a lot for this!!

[gunters63]

@Andarist: Btw, I found this interesting issue in the typescript tracker: microsoft/TypeScript#50482
use-gesture is not marked as "module" so we can get away with missing extensions in the .d.ts files here I guess :)

[dbismut]

Hey @Andarist @gunters63 hope you're well and having a nice Christmas break.
I was made aware that this sandbox doesn't work properly on Codesandbox since last upgrade.

https://codesandbox.io/s/github/pmndrs/use-gesture/tree/main/demo/src/sandboxes/card-zoom

It displays a warning related to importing dragAction and pinchAction, which are exported variables from @use-gesture/core/actions:

import { createUseGesture, dragAction, pinchAction } from '@use-gesture/react'

The lib uses those variables in order to create custom gestures.

Not sure what happens, as when I download the sandbox locally (from csb) it seems to be working normally. When downgrading to 10.2.22 the sandbox seems to work again.

[Andarist]

I believe that this is the very same problem that I was reporting to the CodeSandbox team a couple of months ago (thread in their Discord). It seems that the issue has not been resolved.

The problem is that the bundler~ that they are using for their online codesandboxes has bugs when it comes to package resolving. That's why it works locally - since then you are using webpack directly and it handles this situation correctly.

I would recommend reaching out to the CodeSandbox team again, letting them know about this issue and hoping that they find the time to fix this. When I reported this, I was looking for a place to fix this but IIRC I've concluded that it's in their private repos so I couldn't do much about this myself.

[dbismut]

@Andarist thanks for the detailed explanation. When I look for the @use-gesture/core module in the Sources tab from Chrome inspector:

this is what I see locally:

this is what I see on csb:

I don't know if this is related but indeed something is different.

[Andarist]

Yeah, exactly - as you can see on CodeSandbox a mix of CJS and ESM files was loaded. This creates a "dual package problem". You can read more about dual package hazard here. I call it a problem here since it's not a hazard anymore, it created a problem :P.

However, this just shouldn't happen here. There is no hazard in the package setup and thus the problem should not occur. The broken resolution algorithm in CodeSandbox creates that hazard and thus the problem. This looks exactly like what I reported.

[gunters63]

I find it odd that Codesandbox transpiles App.tsx always to CJS, even when you set "type": "module", in package.json.
This is not the first time I had problems like this in Codesandbox too. The Javascript world should really move forward to ESM now...

[gunters63]

Yes, and with 10.2.22 (without the exports) Codesandbox only loads the ESM modules from use-gesture:

[Andarist]

I think that it's best to wait till January and then try to ping Ives/CSB team about this issue through the appropriate support channels.

[dbismut]

Guys I'm not sure what to do. Half of my interactions on use-gesture are about the lib not working on CSB. It's quite a big deal, I can't really ignore the fact that usage with CSB is probably greater than custom module resolution settings for typescript (please prove me wrong!)

[gunters63]

Its really unfortunate that CSB is hindering adopting new Javascript standards.

The user base relying on exports is surely much smaller (Maybe I am the only one :D). I could live with a rollback, I use pnpm and could use its patch feature in my repository.

Another solution could probably be to release a second module use-gesture-nodenext on npm which contains the package exports, the normal use-gesture package would have the old-style package.json. This should probably be quite easy to do.

[Andarist]

There is also a chance that adding exports to the "nested" package.json files (such as this one) would solve the problem.

This shouldn't be needed - and in fact, that could be very misleading. exports are only read from the root package.json file. The only thing that should be read from the "closest" one is the type property. I've seen bundlers misimplementing this though - and perhaps that's the case here. So there is a chance that we could, temporarily, make their broken implementation work.

[dbismut]

One last huge thanks to @Andarist that helped Codesandbox fix this! Case closed.