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

feat: semver-minor bump for publishing with "main" #64

Merged
merged 1 commit into from
Mar 8, 2021

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Mar 6, 2021

Meaningless edit to get a new auto-publish to happen with the latest ipjs that will publish with a "main" pointing to the CJS entrypoint to support esbuild.

I'm doing this as a semver-minor because it's not a major but I'd like it to have more signal than just a patch and make it easier to exclude if someone has problems (~ instead of ^ .. I know, it's weak, but it's something and I'm just hesitant to roll something like this out as if it's a trivial patch when we really don't know what bundler or loader this might disrupt).

@rvagg rvagg merged commit 309eb22 into master Mar 8, 2021
@rvagg rvagg deleted the rvagg/publish-with-main branch March 8, 2021 00:41
@rvagg
Copy link
Member Author

rvagg commented Mar 8, 2021

OK, so we did break something, but it was TypeScript! I didn't expect that'd be the one to break. After trying to consume this in @ipld/car the types check does this:

lib/decoder.js:3:21 - error TS2307: Cannot find module 'multiformats' or its corresponding type declarations.

3 import { CID } from 'multiformats'
                      ~~~~~~~~~~~~~~

So I guess TypeScript is treating "main" as more authoritative than "exports" (which was the fear, that some bundler / transpiler would do this). We have a "types" definition pointing to a "types/" and it's all there (note that it's dist/types if you do it locally in this repo, but dist/ is what's published). But the "main" points in to cjs/... which seems to be overriding it.

rvagg added a commit that referenced this pull request Mar 8, 2021
Ref: #64 (comment)

Removing `npx` from the workflow while I'm at it. This can be added back in if
anyone objects but I'm not a big fan of `npx` for common-path scripts.
rvagg added a commit that referenced this pull request Mar 8, 2021
Ref: #64 (comment)

Removing `npx` from the workflow while I'm at it. This can be added back in if
anyone objects but I'm not a big fan of `npx` for common-path scripts.
rvagg added a commit that referenced this pull request Mar 8, 2021
Now we're publishing a `"main"`, TypeScript is using that rather than the
`"exports"` map. It points to the CJS published version, located in cjs/src
in the bundle. This adds a descriptor that tells TypeScript to look for types
for that location in the types/, which should be the same as to what it finds
in the export map.

Ref: #64 (comment)
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.

1 participant