plugins icon indicating copy to clipboard operation
plugins copied to clipboard

fix: add types to the exports map

Open RebeccaStevens opened this issue 3 years ago • 7 comments

Rollup Plugin Name: commonjs, node-resolve, and pluginutils

This PR contains:

  • [ ] bugfix
  • [ ] feature
  • [ ] refactor
  • [ ] documentation
  • [x] other

Are tests included?

  • [ ] yes (bugfixes and features will not be merged without tests)
  • [x] no

Breaking Changes?

  • [ ] yes (breaking changes will not be merged unless absolutely necessary)
  • [x] no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

Allows types to be imported when using "moduleResolution": "Node16" in TypeScript.

~This PR may need to be modified based on the discussion here: https://github.com/microsoft/TypeScript/issues/49299~ - Updates made.

RebeccaStevens avatar May 29 '22 09:05 RebeccaStevens

I'm in favor of the change, but having to manually maintain two separate type files is additional burden on an already lean squad of maintainers. I'd like to see this automated if at all possible before we pull the trigger on the change.

Looks like there's a CI failure there too. If that's not reproducible locally, please let us know.

shellscape avatar May 29 '22 19:05 shellscape

With resolution mode assertions that are available in typescript@next, the duplication may be able to be removed.

// index.d.mts
export type { Foo, Bar } from "./index.js" assert { "resolution-mode": "require" };

RebeccaStevens avatar May 30 '22 07:05 RebeccaStevens

I think the .d.ts version of the types needs to use export = ... syntax instead of export default ... I tried to do something similar in https://github.com/auth0/jwt-decode/pull/130, the way to reduce duplications is too extract the common types into a common.d.ts file. Well it's the best i could come up with

perrin4869 avatar Jun 02 '22 01:06 perrin4869

@perrin4869 .d.mts and .d.cts cannot import from one another without using resolution mode assertions which are unstable in TS 4.7.

Note: .d.ts files will either be treated as .d.mts or .d.cts depending on the value of the type field in package.json. They don't act as a third type that can be imported by either.

RebeccaStevens avatar Jun 02 '22 04:06 RebeccaStevens

@RebeccaStevens It does seem to work in my other PR though, at least as far as I tested. I thought the main incompatibility between the two types of modules is how export default vs export = works? From what I can tell importing named exports seems to work fine between the two module types...

perrin4869 avatar Jun 02 '22 05:06 perrin4869

This PR hasn't seen any activity in over 60 days now. Because it's already been reviewed, I'd really rather not close it as abandoned. My concern about two separate type files still exists. If TS 4.7 resolves that, then I think changes to that effect can allow this to move forward.

shellscape avatar Aug 09 '22 14:08 shellscape

Is there a workaround in the meantime? It's a big shame that the types are in the package but can't be used in "type": "module" mode.

wycats avatar Aug 16 '22 22:08 wycats

Closing as abandoned.

shellscape avatar Nov 27 '22 17:11 shellscape