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

Remove types field in package.json #75

Merged
merged 1 commit into from
May 20, 2021
Merged

Remove types field in package.json #75

merged 1 commit into from
May 20, 2021

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented May 19, 2021

The types folder is not published. I think this should do it.

@axelboc
Copy link
Contributor Author

axelboc commented May 19, 2021

Not sure if files in package.json is needed at all, since there's already an .npmignore file. Feel free to disregard this PR if that's the case.

@xobotyi
Copy link
Contributor

xobotyi commented May 19, 2021

Woops =)
But the issue is different. types folder not generated anymore, per-file .d.ts files used instead.
types field of package.json and !/types line in .npmignore is obsolete and should be removed. If not difficult - do it please.

files directive used in some IDEs and bundlers so it is better to have it.

@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #75 (283b888) into master (2a20eb8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #75   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          303       303           
  Branches        58        58           
=========================================
  Hits           303       303           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a20eb8...283b888. Read the comment docs.

@xobotyi xobotyi force-pushed the master branch 3 times, most recently from 8dc0d88 to 2a20eb8 Compare May 20, 2021 00:33
@axelboc
Copy link
Contributor Author

axelboc commented May 20, 2021

But the issue is different. types folder not generated anymore, per-file .d.ts files used instead. types field of package.json and !/types line in .npmignore is obsolete and should be removed.

Doesn't TypeScript need the types field in package.json to point to an index.d.ts entrypoint of sorts? At the moment, when I try to import a hook in my project, VSCode gives me this error: Could not find a declaration file for module '@react-hookz/web'.

@xobotyi
Copy link
Contributor

xobotyi commented May 20, 2021

@axelboc it does not, in case of absense of types field it searches typings right next to files.

Separate types folder breaks the hint about multiversion (cjs,esm,esnext) for some IDEs (idea 4ex), this is why i switched from it.

As for now you have an error since there is no types folder.

@axelboc
Copy link
Contributor Author

axelboc commented May 20, 2021

Right, I get you, thanks for the explanation 😊

The `types` folder is not generated anymore, per-file .d.ts files are used instead.
@axelboc axelboc changed the title Fix types not being published Remove types field in package.json May 20, 2021
@axelboc axelboc changed the title Remove types field in package.json Remove types field in package.json May 20, 2021
@xobotyi xobotyi merged commit 4dd7f06 into react-hookz:master May 20, 2021
@axelboc axelboc deleted the patch-1 branch May 20, 2021 07:34
xobotyi pushed a commit that referenced this pull request May 20, 2021
The `types` folder is not generated anymore, per-file .d.ts files are used instead.
github-actions bot pushed a commit that referenced this pull request May 20, 2021
## [1.20.1](v1.20.0...v1.20.1) (2021-05-20)

### Bug Fixes

* remove `types` field in package.json ([#75](#75)) ([340e7d7](340e7d7))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants