Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type exports to package.json #40

Closed
wants to merge 1 commit into from

Conversation

mrdrogdrog
Copy link

@mrdrogdrog
Copy link
Author

bump

@dmonad
Copy link
Owner

dmonad commented Nov 2, 2022

The types are located in the root folder next to the original files. Hence, it shouldn't be necessary to explicitly link to the files..

Before you make another PR, can you please explain how your typescript version doesn't find the type definitions?

@dmonad dmonad closed this Nov 2, 2022
@mrdrogdrog
Copy link
Author

mrdrogdrog commented Nov 2, 2022

Sure.

The lib i'm building is a module. Therefore all imports have to be precise. When I import e.g. "array" the linker doesn't look for a "array.js". It looks for an export statement in the package.json that matches "array". It doesn't assume things anymore.
Well and this is your package.json:

    "./array.js": "./array.js",
    "./dist/array.cjs": "./dist/array.cjs",
    "./array": {
      "import": "./array.js",
      "require": "./dist/array.cjs"
    },

If I try to import just "array" it uses the block at the bottom. But this block doesn't contain the type file reference. Therefore typescript can't find type definitions because the linker doesn't assume anything anymore.

Of course I could just import "array.js" instead of the alias export statement. The entry for this string is pointing directly to a .js file instead of a block which allows the linker to see the file and therefore look for a .d.ts next to it... But then I can't import libs which just import "array" because then I'm gonna have the same problem. (to be precise: I have this problem with y-protocols which imports lib0/encoding 🙃 (another reason why I've created yjs/y-protocols#21) ).

@mrdrogdrog
Copy link
Author

mrdrogdrog commented Nov 2, 2022

So I ask you to please merge this pull request. Otherwise I can't use this lib (and y-protocols) with esmodule libs and would need to replace it. :/

@mrdrogdrog
Copy link
Author

mrdrogdrog commented Dec 2, 2022

Did you have a chance to take a look at this again?
Otherwise I'm gonna need to replace lib0 and y-protocols soon.

@dmonad
Copy link
Owner

dmonad commented Dec 2, 2022

@mrdrogdrog

As I said before, the type declarations are located next to the original files. So "./dist/array.d.ts"` doesn't exist. This PR is untested and will break everyone else.

So before you make another PR. Please explain why the current approach doesn't work. What is your setup? Why does it work for everyone else but you?

Second, you need to change your PR to point to the actual type declarations: ./array.d.ts not ./dist/array.d.ts

Third, you need to test the PR and ensure it actually works. Please also show me how I can verify that it works in the future.

@mrdrogdrog
Copy link
Author

mrdrogdrog commented Dec 2, 2022

As I said before, the type declarations are located next to the original files. So "./dist/array.d.ts"` doesn't exist.

Nope. they exist.

image

But okay. Since you prefer the top level type files I changed the code.

This PR is untested and will break everyone else.

No. I've tested it locally and it works. It doesn't break anything since the export object is only used if typescript is using the ESM resolver.

So before you make another PR. Please explain why the current approach doesn't work. What is your setup? Why does it work for everyone else but you?

Like I explained before here a full module setup relies only on the export object and doesn't assume the location of type files.

I guess most of the people use the default configuration of typescript for module resolution which is the commonjs module resolver which still assumes the location of type files.
My project(s) (at least the libraries) use rollup to create MJS and CJS exports. I create these libs in typescript and have configured typescript to produce ESM files. Then I use rollup (microbundle to be precise) to generate cjs code. I do this by telling typescript to use the newer module resolver which relies on the exports object. Therefore the dependencies have to be either commonjs packages or export the type declaration files.

image

I recorded this quick video to demonstrate the effect of the modified package.json . I used my lib's test cases because the tests are also written in typescript and therefore also use the ESM resolver.

lib0-2022-12-02_12.31.35.mp4

@mrdrogdrog
Copy link
Author

mrdrogdrog commented Dec 2, 2022

btw: yjs exports the types correctly https://github.com/yjs/yjs/blob/main/package.json#L29 . That's why this lib doesn't produce such errors.

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

Successfully merging this pull request may close these issues.

2 participants