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

fix: update package.json to make types work #15

Merged
merged 2 commits into from
Mar 22, 2024

Conversation

florian-lefebvre
Copy link

No description provided.

@RodrigoTomeES
Copy link
Owner

RodrigoTomeES commented Mar 21, 2024

@florian-lefebvre hi, thanks so much for the PR!!!

But I tested it and it doesn't work, but I found the problem. The route of the types was wrong 🤦🏼‍♂️, it is "types": "./dist/src/index.d.ts",. But what is the difference between declare the main and the types and your declaration form?

@florian-lefebvre
Copy link
Author

I'm not sure tbh! @Fryuni do you know the difference?

@Fryuni
Copy link

Fryuni commented Mar 22, 2024

From the docs:

The "main" field is supported in all versions of Node.js, but its capabilities are limited: it only defines the main entry point of the package.

The "exports" provides a modern alternative to "main" allowing multiple entry points to be defined, conditional entry resolution support between environments, and preventing any other entry points besides those defined in "exports". This encapsulation allows module authors to clearly define the public interface for their package.

They work mostly the same, but "exports" is more flexible and when present prevents importing anything that is not declared.

I don't know what was the original problem this PR was trying to solve, but this change should not affect anything importing the library root (import ... from 'astro-rename';)

@florian-lefebvre
Copy link
Author

Gotcha thank you! Yeah this PR does not fix the initial problem (but @RodrigoTomeES figured it out) but I believe it doesn't hurt looking at your explanation. Up to you @RodrigoTomeES!

@RodrigoTomeES
Copy link
Owner

@Fryuni thanks for the explanation very interesting!!. @florian-lefebvre I will use your approach, it appear to be a more modern way to do it and better because it only import exactly what I export explicitly

@RodrigoTomeES RodrigoTomeES merged commit 676940f into RodrigoTomeES:feature/v2 Mar 22, 2024
6 checks passed
@florian-lefebvre florian-lefebvre deleted the patch-1 branch March 22, 2024 15:57
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.

3 participants