fp-ts icon indicating copy to clipboard operation
fp-ts copied to clipboard

Drop CJS

Open gcanti opened this issue 1 year ago • 34 comments

Linking the same issue in effect-ts.

As you may already know, there's an ongoing effort to find common ground with effect-ts and unify the ecosystem.

Effect will be ESM only from 31/12/2022.

I propose to switch to ESM only (fp-ts v3 and its alpha releases).

gcanti avatar Sep 16 '22 04:09 gcanti

I suggested that several months ago and @mikearnaldi told me it's absolutely a no-go 😅 It's great he changed his mind. I am all for ESM only because of my own experiences writing a lib in TypeScript with a few Web workers, several dependencies, etc. I finally managed to configure ESM by not using "bundlers" because it was impossible to do with them (you can't just inline web worker using ESM dependencies, the only bundlers capable of doing that is Webpack, and in Next.js, it works out of the box). Now I am challenging another problem with Node.js. I can't just use ESM only library in Node 16 ESM. So, we must ensure that fp-ts ESM will work in Node.js ESM. Once a lib is written in ESM, Node ESM is a must. You can't import ESM in CJS while you can import CJS in ESM. Slightly off-topic, I am contemplating rewriting my lib to Deno and provide a special build for npm and Node.js. Related discussion https://github.com/gcanti/fp-ts/discussions/1782

steida avatar Sep 17 '22 13:09 steida

I got bundler-free ESM to work from /dist/, but only after basically fixing every import statement:

image

It's a pity that TypeScript can't just add *.js to make things works as they should. It should be part of the fp-ts build process.

Used regular expressions:

SEARCH: (\bfrom\s+["']\..*)(["'])
REPLACE: $1.js$2
FILES TO INCLUDE: dist/es6/*.js

kungfooman avatar Sep 18 '22 06:09 kungfooman

I suggested that several months ago and @mikearnaldi told me it's absolutely a no-go 😅 It's great he changed his mind. I am all for ESM only because of my own experiences writing a lib in TypeScript with a few Web workers, several dependencies, etc. I finally managed to configure ESM by not using "bundlers" because it was impossible to do with them (you can't just inline web worker using ESM dependencies, the only bundlers capable of doing that is Webpack, and in Next.js, it works out of the box). Now I am challenging another problem with Node.js. I can't just use ESM only library in Node 16 ESM. So, we must ensure that fp-ts ESM will work in Node.js ESM. Once a lib is written in ESM, Node ESM is a must. You can't import ESM in CJS while you can import CJS in ESM. Slightly off-topic, I am contemplating rewriting my lib to Deno and provide a special build for npm and Node.js. Related discussion #1782

Indeed it was a no-go, at the time if I recall correctly there wasn't even an LTS of node with ESM out, that said even today it will piss off a lot of people I am fairly sure of that BUT I think we have done enough supporting for so long both and we need a push

mikearnaldi avatar Sep 18 '22 13:09 mikearnaldi

I got bundler-free ESM to work from /dist/, but only after basically fixing every import statement:

image

It's a pity that TypeScript can't just add *.js to make things works as they should. It should be part of the fp-ts build process.

Used regular expressions:

SEARCH: (\bfrom\s+["']\..*)(["'])
REPLACE: $1.js$2
FILES TO INCLUDE: dist/es6/*.js

I don't think the build process should touch imports, we have really 2 choices here:

  1. add .js to every import/export
  2. use fully qualified imports like fp-ts/Array

There was a long discussion in TS about adding .js automatically and it was decided not to do so as TS is no longer accepting any term level change to JS

mikearnaldi avatar Sep 18 '22 13:09 mikearnaldi

There was a long discussion in TS about adding .js automatically and it was decided not to do so as TS is no longer accepting any term level change to JS

Yea, and the joke is on them, what do they expect? Import a TS file with JS extension? Their stance is unproductive and blind to reality...

Anyway, we can't change TS but what I suggested is basically your point (1)... just a little script fixing the imports in dist/es6 once they are generated, so ESM actually works as first-class citizen in any modern browser without any bundler.

kungfooman avatar Sep 18 '22 14:09 kungfooman

@kungfooman haha, I just finished reading all that comments. I think what they want is https://github.com/tc39/proposal-type-annotations so there will be js everywhere in the end.

@mikearnaldi Did you try mts? The advantage should be it enforces suffixes, and no further procession is required, I suppose.

steida avatar Sep 18 '22 14:09 steida

So I tried mts, and as a result, I got weird TypeScript errors in Node using it, jeez...

Btw, TypeScript allows .js extension, so rewriting is not required, only checking.

https://github.com/koskimas/kysely/blob/master/scripts/check-esm-imports.js

Also, I tried this eslint plugin https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/extensions.md, but it does not work as expected for some reason.

steida avatar Sep 18 '22 15:09 steida

@kungfooman haha, I just finished reading all that comments. I think what they want is https://github.com/tc39/proposal-type-annotations so there will be js everywhere in the end.

@mikearnaldi Did you try mts? The advantage should be it enforces suffixes, and no further procession is required, I suppose.

I did but the d.ts emitted as d.mts are very bothering to me, also it compiles to mjs not js.

Overall the best set of tradeoffs I found is by using fully qualified imports.

mikearnaldi avatar Sep 18 '22 15:09 mikearnaldi

@mikearnaldi https://github.com/gcanti/fp-ts/issues/1777#issuecomment-1250327708

steida avatar Sep 18 '22 15:09 steida

There was a long discussion in TS about adding .js automatically and it was decided not to do so as TS is no longer accepting any term level change to JS

Yea, and the joke is on them, what do they expect? Import a TS file with JS extension? Their stance is unproductive and blind to reality...

Anyway, we can't change TS but what I suggested is basically your point (1)... just a little script fixing the imports in dist/es6 once they are generated, so ESM actually works as first-class citizen in any modern browser without any bundler.

Yes, the 'little script' may break other things though, it's not a really good practice to run regex replace on code, if any a tsc plugin or something of the sort of an ast plugin (if using swc/esbuild) would be better. Also no the joke is not on them, it's what has been decided and there was a long enough explanation about why that is needed, to me the bad decision is upfront in js not really in ts as again when writing ts you write term valid js (design goal)

mikearnaldi avatar Sep 18 '22 15:09 mikearnaldi

@mikearnaldi #1777 (comment)

What do you mean by 'ts allows js'?

mikearnaldi avatar Sep 18 '22 15:09 mikearnaldi

You can use .js extension in .ts file. It looks weird, but tsc compiles it properly. I suppose it's future compatible when https://github.com/tc39/proposal-type-annotations became a reality. Then we will rename all .ts files to .js, and everything will be nice and all again 😂

@mikearnaldi You did not get it; that script does not rewrite anything; it's only a check.

steida avatar Sep 18 '22 15:09 steida

@mikearnaldi https://github.com/microsoft/TypeScript/issues/16577#issuecomment-754941937 ".js file extensions are now allowed"

steida avatar Sep 18 '22 15:09 steida

@steida yes I am aware, it was part of the esm integration, as of my mention of a script I was not referring to the script you posted but to the proposal of 'It should be part of the fp-ts build process.' by @kungfooman which I don't agree with, I think as mentioned above that we have 2 choices either stay with relative imports but adding the '.js' extension or use fully qualified imports that use the package.json lookup logic

mikearnaldi avatar Sep 18 '22 16:09 mikearnaldi

Meanwhile, I tested two eslint plugins for checking file extensions. Guess what, no one works.

This one check also packages, can't be disabled. https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/file-extension-in-import.md

This does not work either. https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/extensions.md

steida avatar Sep 18 '22 16:09 steida

Funny thing [email protected] with Node 16 ESM:

I need pipe. I can't use import { pipe } from "fp-ts/function" of course. But I can't use import { pipe } from "fp-ts/es6/function.js" either.

server:start: file:///Users/danielsteigerwald/dev/evolu/apps/server/dist/index.js:1
server:start: import { pipe } from "fp-ts/es6/function.js";
server:start:          ^^^^
server:start: SyntaxError: Named export 'pipe' not found. The requested module 'fp-ts/es6/function.js' is a CommonJS module, which may not support all module.exports as named exports.

For some reason, importing from lib works import { pipe } from "fp-ts/lib/function.js"

And it's transitive, so I suppose many fp-ts libraries will not work with Node 16 ESM.

And because my library is ESM only, I can't import it as CommonJS.

(node:39713) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

I hope fp-ts 3 will be released soon 🙂

steida avatar Sep 18 '22 20:09 steida

Funny thing [email protected] with Node 16 ESM:

I need pipe. I can't use import { pipe } from "fp-ts/function" of course. But I can't use import { pipe } from "fp-ts/es6/function.js" either.

server:start: file:///Users/danielsteigerwald/dev/evolu/apps/server/dist/index.js:1
server:start: import { pipe } from "fp-ts/es6/function.js";
server:start:          ^^^^
server:start: SyntaxError: Named export 'pipe' not found. The requested module 'fp-ts/es6/function.js' is a CommonJS module, which may not support all module.exports as named exports.

For some reason, importing from lib works import { pipe } from "fp-ts/lib/function.js"

And it's transitive, so I suppose many fp-ts libraries will not work with Node 16 ESM.

And because my library is ESM only, I can't import it as CommonJS.

(node:39713) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

I hope fp-ts 3 will be released soon 🙂

I don't think v2 was meant to be compatible with ESM as it doesn't respect the following: a .js file is identified as CJS if the nearest package.json doesn't have type: "module" and a .js file is identified as ESM if the nearest package.json is a module.

In effect to have both working I use:

  1. fully qualified imports so no .js extension added but @effect/core/...
  2. a pipeline that produces cjs in the main package root (compat with pre-exports clients like old node)
  3. a pipeline that produces esm that also renames files to .mjs and because I don't use relative imports they need no change

to have the same working with relative imports it would be needed to change all the relative imports to point to the correct mjs/js extension

At the moment v2 is only ok for bundlers

mikearnaldi avatar Sep 19 '22 08:09 mikearnaldi

@mikearnaldi Can you please repeat the problem with writing .mts from the start? It enforces the extension and should work well. I had some issues with types, but the same issues I had with .ts so the problem was elsewhere I guess.

steida avatar Sep 19 '22 11:09 steida

@mikearnaldi Can you please repeat the problem with writing .mts from the start? It enforces the extension and should work well. I had some issues with types, but the same issues I had with .ts so the problem was elsewhere I guess.

I don't quite like to have mts types they are incompatible with older ts and yes it emits mjs which are fine but if I want mjs in the extension it is usually for cases where both mjs and cjs needs to be emitted and that means always an additional pipeline to fix up the imports, instead in a pure module (type module) plain js extensions are fine. So to summarise I don't dislike mts / cts but I just don't have any situation where they actually help, they would help a lot in a hybrid package where you keep state in cjs and have mjs wrappers to support importing both in the same project (mjs can import cjs and you may want to share state) but I don't really suggest that as a good idea I think we should migrate to a world where we only have one (esm)

mikearnaldi avatar Sep 20 '22 09:09 mikearnaldi

As for testing... I just spend a few hours trying to force Jest to work with ESM. No success. And I'm not alone.

  • https://twitter.com/kcaliskan/status/1572463967603429377
  • https://twitter.com/thdxr/status/1572597312664354816

Then I switched to Vitest (like the others). It worked immediately without any configuration.

steida avatar Sep 21 '22 18:09 steida

So how many hours did you waste now by doing it "properly" by testing all kinds of plugins with extra bells and whistles?

I got ESM to work with ONE regex 😂

Good job I guess, professionals...

kungfooman avatar Sep 22 '22 08:09 kungfooman

So how many hours did you waste now by doing it "properly" by testing all kinds of plugins with extra bells and whistles?

I got ESM to work with ONE regex 😂

Good job I guess, professionals...

I have ESM working since ages with absolutely no workaround. Also we're all here to help each other, that kind of comment you can keep for yourself.

mikearnaldi avatar Sep 22 '22 22:09 mikearnaldi

I will recommend still building to .js and .d.ts files and using type: module.

There seems to be better support for extensionless imports with .js and .d.ts files. If you have sections under exports that use wildcards and also point to files with the new explicit extensions. Then they seem to require extensions when importing.

I don't know if this is just a moduleResolution: NodeNext thing or what. But I've noticed this when importing some packages that use .mjs and .d.mts.

viell-dev avatar Oct 17 '22 12:10 viell-dev

I'm using "moduleResolution": "nodenext" with "type": "module". At the moment I am getting the following error:

import { pipe } from "fp-ts/function";
         ^^^^
SyntaxError: Named export 'pipe' not found. The requested module 'fp-ts/function' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'fp-ts/function';
const { pipe } = pkg;

My version of fp-ts is 3.0.0-alpha.26.

azat-io avatar Nov 02 '22 23:11 azat-io

Try with @fp-ts/core we are migrating to an org structure and the new fp-ts will be published there

On Wed, 2 Nov 2022, 23:54 Azat S., @.***> wrote:

I'm using "moduleResolution": "nodenext" with "type": "module". At the moment I am getting the following error:

import { pipe } from "fp-ts/function"; ^^^^ SyntaxError: Named export 'pipe' not found. The requested module 'fp-ts/function' is a CommonJS module, which may not support all module.exports as named exports. CommonJS modules can always be imported via the default export, for example using:

import pkg from 'fp-ts/function'; const { pipe } = pkg;

My version of fp-ts is 3.0.0-alpha.26.

— Reply to this email directly, view it on GitHub https://github.com/gcanti/fp-ts/issues/1777#issuecomment-1301502136, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFZAKCX6DSV754HPCKX5SPLWGL5LHANCNFSM6AAAAAAQN7KVYQ . You are receiving this because you were mentioned.Message ID: @.***>

mikearnaldi avatar Nov 02 '22 23:11 mikearnaldi

@mikearnaldi It works! Thanks!

Off topic: Why are all the useful things in @fp-ts/data? What is @fp-ts/core for?

azat-io avatar Nov 03 '22 00:11 azat-io

@mikearnaldi It works! Thanks!

Off topic: Why are all the useful things in @fp-ts/data? What is @fp-ts/core for?

You're welcome!

So the idea is to separate a bit the implementations to keep it more maintainable and focused, in /core we have only abstractions that general but that at the same time are rarely useful from the user perspective, a normal user will interact mostly with specific packages, the plan is:

  • @fp-ts/data => all the common data structures
  • @fp-ts/codec => similar to io-ts focused on domain modelling
  • @fp-ts/optic => similar to monocle
  • @effect/io => effect execution
  • @effect/stream => streaming focused
  • etc

You can check the repositories in the orgs, in the one for /core there is some kind of initial explainer of what we have in mind:

  • https://github.com/fp-ts
  • https://github.com/Effect-TS

mikearnaldi avatar Nov 03 '22 01:11 mikearnaldi

I will recommend still building to .js and .d.ts files and using type: module.

There seems to be better support for extensionless imports with .js and .d.ts files. If you have sections under exports that use wildcards and also point to files with the new explicit extensions. Then they seem to require extensions when importing.

I don't know if this is just a moduleResolution: NodeNext thing or what. But I've noticed this when importing some packages that use .mjs and .d.mts.

Yeah the mts extension is kind of problematic, with @fp-ts/core we are currently publishing a dual setup with plain .js in the root indexed in a way that is compatible with older systems, then via the exports package we expose the .mjs modules for esm usage which maximise compatibility with every possible setup.

Unfortunately using type: module is still not widely compatible, the whole thing is very messy

mikearnaldi avatar Nov 03 '22 01:11 mikearnaldi

I'd say that "type": "module" is still very intrusive - you're forced convert whole codebase to ESM. Even .babelrc.mjs configs with "Experimental babel async workersfor@babel/register` etc.

I'm still dual-building projects with a mix of babel-preset-typescript due to hard dependencies on babel-macro in certain conditions... and code coverage issues with type-coverage and C8.

Having a "type": "commonjs" project with occasional isolated "exports": { ... }, for a "type": "module" dependency, is fine. So, if fp-ts would really want to switch to "type": "module", I'd stick to the official Isolated modules recommendation first, for the nodenext resolver.

yuriy-yarosh avatar May 28 '23 16:05 yuriy-yarosh

I'd say that "type": "module" is still very intrusive - you're forced convert whole codebase to ESM.

You don't even need "type": "module", just use file extension *.mjs. But if you set it, you can simply drop out via file extension *.cjs. I don't know what's intrusive about that, just use proper file extensions.

IMO main issue is still TypeScript, just rewrite everything in JS with JSDoc and get rid of the ideological TS BS... this also makes the JS hidden under the pile of interspersed types readable again. :joy:

kungfooman avatar May 29 '23 07:05 kungfooman