[css-tokenizer] add types entry in package.json for older projects

ssttevee commented 1 month ago

namely where compilerOptions.moduleResolution < "node16" in tsconfig.json

romainmenke commented 1 month ago

Hi @ssttevee,

That is not a bug. We do not support types for commonjs.

see :

ssttevee commented 1 month ago

This is not about commonjs, it's more about compatibility.

Merely including this field does not support commonjs, but gives bundlers and typescript more options to find the type definitions if there are other configuration constraints that prevent reading from the exports.

Edit: I apologize if this has already been thoroughly discussed elsewhere, but it seems to me that it is intentionally breaking compatibility unnecessarily.

romainmenke commented 1 month ago

Commonjs is supported. Only types for commonjs are not.

If you get a warning about types, they you are actually generating commonjs.

We can't generate types that are correct for both esm and cjs. This has been explained in detail in the linked issues.

So this is not a compat issue, commonjs works. Please don't imply that we are intentionally breaking things when we go above and beyond to provide the best possible software, for free.

You just don't get types, which is not blocking, you can silence the warning about the types. We also have API docs to aid you in using these packages.

ssttevee commented 1 month ago

You're right, I did not fully understand the situation and I apologize. However, I would like to add my data point.

For reference, my project does not use commonjs, but generates esm bundles with what I suppose is commonjs configurations in both my package.json and tsconfig.json. I could be using it completely wrong, but I believe this setup was rather common near the introduction of esm in nodejs.

The bundler automatically includes the esm source if available and falls back to cjs with a wrapper. We have not run into any commonjs issues related to type definitions using this setup.

I was able to rollback to v2.2.1 for the type definitions.

romainmenke commented 1 month ago

For reference, my project does not use commonjs, but generates esm bundles with what I suppose is commonjs configurations in both my package.json and tsconfig.json

🤔 If your project really is using and generating esm, then you should have working types. Can you share your configuration?

Or even better submit a PR with a minimal repro?

We have some e2e tests to ensure that TypeScript works, for example this one : https://github.com/csstools/postcss-plugins/tree/main/e2e/typescript/esm

So either your project isn't generating esm, which really is easy to do by accident, or we do have a bug somewhere.

ssttevee commented 1 month ago

There is actually no problem with the bundling. My deployment environment does not support commonjs, so the generated bundle is definitely esm.

The only problem is the tooling. Because my moduleResolution is not node16, typescript doesn't read the exports and the type checking fails. I'm fairly certain this is an artifact of the bare minimum effort to make a pre-esm project to target an esm environment.

I don't think I'll have time to build a minimal repro, but here is my tsconfig:

{
  "compilerOptions": {
    "allowJs": true,
    "target": "ES2017",
    "module": "esnext",
    "lib": [
      "es2020",
      "DOM",
      "WebWorker",
      "DOM.Iterable"
    ],
    "jsx": "react-jsx",
    "jsxImportSource": "@builder.io/qwik",
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "resolveJsonModule": true,
    "moduleResolution": "node",
    "esModuleInterop": true,
    "skipLibCheck": true,
    "incremental": true,
    "isolatedModules": true,
    "outDir": "tmp",
    "noEmit": true,
    "types": [
      "node",
    ],
    "paths": {
      "~/*": [
        "./src/*"
      ]
    }
  },
  "include": [
    "src",
  ],
}

I have also tried using node16 module resolution, but that breaks more packages than it fixes.

romainmenke commented 1 month ago

Do you have "type": "module" in your package.json or are you using the .mts file suffix? If you don't do either TypeScript will still act as if you are writing commonjs.

ssttevee commented 1 month ago

Nope, and that's exactly the problem.

I tried renaming the file to .mts but that does not seem to change the module resolution.

romainmenke commented 1 month ago

https://www.typescriptlang.org/tsconfig/#moduleResolution

'node10' (previously called 'node') for Node.js versions older than v10, which only support CommonJS require. You probably won’t need to use node10 in modern code.

So you are still effectively using commonjs, which we don't have types for :)

So this definitely is an issue with your config.

csstools / postcss-plugins

[css-tokenizer] add types entry in package.json for older projects #1437