Drop CJS support

[frandiox]

Currently, we are compiling Hydrogen to both CJS and ESM. It would be much simpler and faster to use only ESM.

In tsconfig.cjs.json we explicitly include src/framework to be compiled to CJS. However, the whole project ends up being compiled to CJS because in src/framework we are importing types from other folders (and TSC follows imports). This has two issues:

1. It's slower: we compile files to CJS that we are not using later.
2. It can break: targeting CJS in files that are supposed to be ESM can crash the TSC process. For example, it breaks when encountering import.meta (cc @cartogram )

We need CJS at the moment because Vite compiles "import" statements in vite.config.js to "require" in projects (user apps) that don't have "type": "module" in package.json. Therefore, everything that is imported in vite.config.js (plugins) needs to be in CJS.

If we want to use ESM only, we would need to add "type": "module" to both Hydrogen package and Hydrogen apps. In this scenario, Vite would not use "require" in vite.config.js. However, this has two problems:

1. ESM complains about missing file extensions. This can be worked around by using the flag --experimental-specifier-resolution=node when running Vite.
2. ESBuild crashes due to this issue. No workaround so far.

[benmccann]

ESBuild crashes due to evanw/esbuild#1921. No workaround so far.

It looks like there's a PR pending to fix it: evanw/esbuild#2067

[frehner]

Vite appears to have just merged a workaround for the ESBuild issue above.

[frehner]

I was looking at going through the codebase and adding all the exact file extensions to address issue 1 above, but I think I'm running into an issue here:

Take this export from src/index.ts for example:

export {useShop} from './foundation/useShop';

If we want to add the complete file extension so that it's an actual file extension:

// TS error: An import path cannot end with a '.ts' extension. Consider importing './foundation/useShop/index.js' instead. ts(2691)
export {useShop} from './foundation/useShop/index.ts';

So we can't end the file extension with .ts.

Ok, so what if we end it in .js, which appears to be the recommended TS way? (There are a TON of issues in the TS github repo about this, but here's a recent example of it: microsoft/TypeScript#49083 )

// "./foundation/useShop/index.js" is not found. eslint[node/no-missing-import](https://github.com/mysticatea/eslint-plugin-node/blob/v11.1.0/docs/rules/no-missing-import.md)
export {useShop} from './foundation/useShop/index.js';

ESLint isn't happy about the fact that you're trying to import a non-existent file, which is quite reasonable to be honest.

I explored this because I wanted to see if we could address issue 1 without using an experimental flag, but the solution seems to be either use the flag, or disable ESLint's error checking for this type of error so that TS can have its way.

🤷

[frandiox]

@frehner Thanks for trying that! Yeah, I think we will need to disable this error in ESLint. I don't like using .js when we have TS but I'm aware that the TS team recommends it so... 🤷

The experimental flag will be removed at some point so we should not rely on it...

Do not rely on this flag. We plan to remove it once the Loaders API has advanced to the point that equivalent functionality can be achieved via custom loaders.

[juanpprieto]

I actually don't mind using .js for export index files. Makes it easy to spot exports vs actual components/utils..

[blittle]

But what if those index files are removed? That's been a common discussion, especially in the framework code.

[lordofthecactus]

I'm not a fan of the file extension .js when the dev is working with .ts files. Seems to me like a possible cause of confusion.

[frehner]

I'm not a fan of the file extension .js when the dev is working with .ts files. Seems to me like a possible cause of confusion.

Agreed, I don't think anyone likes it. But it must be done that way, sadly.