search

bluwy / publint

Lint packaging errors
https://publint.dev
MIT License
932 stars 19 forks source link

Shared "types" condition for "import" and "require" should not warn if has no default export #63

Closed bluwy closed 11 months ago

bluwy commented 11 months ago

https://publint.dev/resolve.exports@2.0.2

There should not be a warning for the "types" condition as it's perfectly safe to be shared for both "import" and "require". The "import" would use the CJS-scoped context when interpreting default exports, which can be wrong for ESM, but if the package doesn't default export things, it's fine.

sapphi-red commented 11 months ago

IIRC the file needs to use file extensions for imports (https://github.com/vitejs/vite/pull/13947) to share the file for them. In case of resolve.exports, it's fine as it doesn't contain any import though.

bluwy commented 11 months ago

True. Yeah the heuristic to skip this warning then is if it has

  1. No imports (relative imports only)
  2. No default export

I'm thinking we can allow sharing if it's only bare imports as it may still work for some deps. Further analysis for the dep could be done in https://github.com/bluwy/publint/issues/18

bluwy commented 11 months ago

I think I confused myself. You could import a package that resolves to a CJS dts, and that dts with import statements should be able to import relatively as long as it has the right extension. But the extension is enforced by node16 and bundler itself?

I think I'd want to skip checking whether an import has the extension or not first, since it's harder to track and convey the error message. It would be a new error message e.g. "the dts file contains imports that does not work in node16/bundler".

So regarding sharing the dts itself,

  1. If import resolves to CJS dts, and has default export, warn as it's ambiguous.
  2. If require resolves to ESM dts, and there's corresponding CJS code, warn as it signals you can dynamic import the entrypoint only, even though there's actual CJS code.

Resources:

  1. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseCJS.md
  2. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseESM.md
sapphi-red commented 11 months ago

Ah, you're right. Sorry for the confusion.

bluwy commented 11 months ago

I'm confusing myself big time 😄 So apparently default export, regardless if one exists or not is also ambiguous, because it's ambiguous at the consumer-side. The attw FalseCJS link already explains that. I'll close this for now.