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

Change path of the types in package.json #377

Merged
merged 1 commit into from
Sep 20, 2024
Merged

Change path of the types in package.json #377

merged 1 commit into from
Sep 20, 2024

Conversation

Pimm
Copy link
Collaborator

@Pimm Pimm commented Sep 19, 2024

It seems that the TypeScript compiler previously put the type definitions into dist/types/src, but now puts them in dist/types. This may have happened during our recent upgrade of TypeScript.

This PR updates package.json to use the new path.

Please do check my work!

It seems that the TypeScript compiler previously put the type definitions into dist/types/src, but now puts them in dist/types. This may have happened here: #360.
@Pimm Pimm requested a review from janpaepke September 19, 2024 22:21
@janpaepke janpaepke added the bug Unexpected behaviour. label Sep 20, 2024
@janpaepke
Copy link
Collaborator

janpaepke commented Sep 20, 2024

I tested your theory by building with the ts version from the previous version and comparing the dist folder structure.
It seems you are indeed correct.

The newer ts version does omit the src in the path, which tbh makes sense from a folder structure point of view.

This is a really good find! I wonder how we can better protect against something like this in the future. Maybe add tsd to the testing routine?

This subltle change will also have consequences for people currently importing types from deep links, which I know you shouldn't do, but people are definitely doing.
This time I'm on the fence wether or not this should be considered breaking, but it doesn't matter much, since this part of the major.

We might still want to document it in the migration/release notes?

@Pimm
Copy link
Collaborator Author

Pimm commented Sep 20, 2024

The newer ts version does omit the src in the path, which tbh makes sense from a folder structure point of view.

It's definitely neat!

This subltle change will also have consequences for people currently importing types from deep links, which I know you shouldn't do, #356.
This time I'm on the fence wether or not this should be considered breaking, but it doesn't matter much, since this part of the major.

We might still want to document it in the migration/release notes?

Using internal APIs comes with that risk. I would personally not consider this breaking, nor would I include it in the migration guide.

@Pimm Pimm merged commit a375534 into master Sep 20, 2024
5 checks passed
@Pimm Pimm deleted the pimm/type-location branch September 20, 2024 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behaviour.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants