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: restore the types field in the package.json file #28

Conversation

sounisi5011
Copy link
Contributor

TypeScript may not use the "types" import condition.

For example, if the value of the moduleResolution option is "node". In this case, TypeScript uses the types field and the "types" import condition is not used.

So even if the "types" import condition is used, package.json still needs the types field for fallback. Therefore, I have restored the types field that was removed in f724043.

Resolves #26

  • I tested this change

    $ pnpm run build && pnpm pack
    $
    $ mkdir test-pr-28 && cd test-pr-28
    $
    $ echo '{}' > ./package.json
    $ echo '{ "compilerOptions": { "moduleResolution": "node", "esModuleInterop": true, "strict": true } }' > ./tsconfig.node.json
    $ echo '{ "compilerOptions": { "moduleResolution": "node16", "esModuleInterop": true, "strict": true } }' > ./tsconfig.node16.json
    $
    $ echo 'console.log(require("used-pm")())' > foo.cjs
    $ echo 'import currentPackageManager from "used-pm"; console.log(currentPackageManager())' > foo.mjs
    $ cp foo.mjs foo.cts
    $ cp foo.mjs foo.mts
    $
    $ npm i typescript ../used-pm-1.0.2.tgz # I am using the npm command by intention. When using pnpm, used-pm 1.0.2 released to npm may be added instead of used-pm-1.0.2.tgz due to global caching
    $
    $ pnpm exec node foo.cjs && pnpm exec node foo.mjs && pnpm exec tsc -p tsconfig.node16.json --noEmit && pnpm exec tsc -p tsconfig.node.json --noEmit

TypeScript may not use the `"types"` import condition.

For example, if the value of the `moduleResolution` option is `"node"`. In this case,
TypeScript uses the `types` field and the `"types"` import condition is not used.

So even if the `"types"` import condition is used, `package.json` still needs the `types` field
for fallback. Therefore, I have restored the `types` field
that was removed in f724043.

Resolves mheob#26
@sounisi5011 sounisi5011 requested a review from mheob as a code owner May 26, 2023 06:17
@sounisi5011
Copy link
Contributor Author

Note
I am not a native English speaker, so I am using the DeepL translator (DeepL Pro) for all English text included in this pull request.

DeepL Pro should have no licensing issues 1, but please check all the English text included (the commit text when merging) before merging.
Maybe the English text contains some mistakes.

If you feel that adding machine-translated text to this repository is undesirable, feel free to close this pull request and create a new one.
I do not take this as an insult.

Footnotes

  1. The problem with using machine translation in OSS is that the translation results may be limited to private use. If the copyright of the translation results belongs to the company providing the machine translation, the user's inclusion of the translation in the OSS may exceed the scope of private use and violate the terms of use of the machine translation and the license of the OSS.

    For example, in the Ubuntu translation project, one person used Google Translate to translate into Japanese, which became a big problem because it violated the Ubuntu license.

    In Wikimedia's opinion, the likelihood of copyright infringement from the use of Google Translate on Wikipedia is "very unlikely".

    In the case of DeepL Pro, the copyright of the translation result belongs to the user.

    7.5 DeepL does not assume any copyrights to the translations made by Customer using the Products. In the event that the translations made by Customer using the Products are deemed to be protected under copyright laws to the benefit of DeepL, DeepL grants to Customer, upon creation of such translations, all exclusive, transferable, sublicensable, worldwide perpetual rights to use the translations without limitation and for any existing or future types of use, including without limitation the right to modify the translations and to create derivative works.
    --- DeepL Terms and Conditions

    Therefore, this problem should not exist.

    However, if you think it is necessary, please rewrite the English text.

Copy link
Owner

@mheob mheob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you very much for your work.

@kodiakhq kodiakhq bot merged commit 963e138 into mheob:main May 29, 2023
@sounisi5011 sounisi5011 deleted the fix-issue-26/restore-types-field-in-package.json branch May 29, 2023 11:31
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.

[BUG]: TypeScript files do not find type declarations when "moduleResolution": "node" is used in tsconfig.json
2 participants