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 types field to package.json exports – fixes ts(7016) #329

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Add types field to package.json exports – fixes ts(7016) #329

merged 1 commit into from
Dec 13, 2023

Conversation

vpctorr
Copy link
Contributor

@vpctorr vpctorr commented Dec 13, 2023

No description provided.

@cosinekitty cosinekitty merged commit 1736b62 into cosinekitty:master Dec 13, 2023
3 of 7 checks passed
cosinekitty added a commit that referenced this pull request Dec 13, 2023
Include TypeScript type definitions in the exported files.
This might be a fix for #294, but I'm not sure.
The only real way to test is to publish on npm and see what happens!
@cosinekitty
Copy link
Owner

Hello @vpctorr and thank you for your contribution. I have merged this change, and I published it in a new npm package, version 2.1.18.

However, during the publish process, I saw a warning that makes me think your change might not have any effect. If it is really removing the "scripts" entry from package.json, it is essentially undoing your change.

don@doctorno:~/github/astronomy/source/js$ npm publish
npm WARN publish npm auto-corrected some errors in your package.json when publishing.  Please run "npm pkg fix" to address these errors.
npm WARN publish errors corrected:
npm WARN publish Removed invalid "scripts"
npm WARN publish "repository.url" was normalized to "git+https://github.com/cosinekitty/astronomy.git"

Also, I would be interested for you to try installing latest (2.1.18) from npm and see if it resolves your problem.

Which, by the way, I am wondering, is it the same problem as #88, or is this something else?

@vpctorr
Copy link
Contributor Author

vpctorr commented Dec 14, 2023

Hey, thanks for the quick merge @cosinekitty – also, the library is just awesome :)

The fix does work perfectly fine! It's related to how TS handles exports since v5.0 (iirc).
#88 tackled a similar issue but for previous TypeScript versions.

As for warning you're getting is strictly related to NPM, which is just complaining about a missing "scripts" field in package.json, although the warning in pretty confusing imo. See npm/cli#6918

#330 should get rid of the warnings, but they have no real impact anyway.

@vpctorr vpctorr deleted the patch-1 branch December 14, 2023 13:14
@cosinekitty
Copy link
Owner

Thank you so much for the explanation. I really appreciate the help with anything node/npm related, because I am definitely not an expert on them!

#330 should get rid of the warnings, but they have no real impact anyway.

Great, I will try this out later today. I will merge it also if the warnings disappear on my system too. Either way, once the code settles down, I will circle back and ask people who ran into #88 to re-test and report results.

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