gcanti / fp-ts

Functional programming in TypeScript
https://gcanti.github.io/fp-ts/
MIT License
10.78k stars 501 forks source link

Drop CJS #1777

Open gcanti opened 2 years ago

gcanti commented 2 years ago

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).

steida commented 2 years ago

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

kungfooman commented 2 years ago

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
mikearnaldi commented 2 years ago

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 commented 2 years ago

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

kungfooman commented 2 years ago

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.

steida commented 2 years ago

@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 commented 2 years ago

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.

mikearnaldi commented 2 years ago

@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.

steida commented 2 years ago

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

mikearnaldi commented 2 years ago

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 commented 2 years ago

@mikearnaldi #1777 (comment)

What do you mean by 'ts allows js'?

steida commented 2 years ago

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 commented 2 years ago

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

mikearnaldi commented 2 years ago

@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

steida commented 2 years ago

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 commented 2 years ago

Funny thing fp-ts@2.12.3 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 πŸ™‚

mikearnaldi commented 2 years ago

Funny thing fp-ts@2.12.3 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

steida commented 2 years ago

@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.

mikearnaldi commented 2 years ago

@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)

steida commented 2 years ago

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

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

kungfooman commented 2 years ago

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...

mikearnaldi commented 2 years ago

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.

viell-dev commented 1 year ago

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.

azat-io commented 1 year ago

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.

mikearnaldi commented 1 year ago

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: @.***>

azat-io commented 1 year ago

@mikearnaldi It works! Thanks!

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

mikearnaldi commented 1 year ago

@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:

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:

mikearnaldi commented 1 year ago

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

yuriy-yarosh commented 1 year ago

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.

kungfooman commented 1 year ago

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:

yuriy-yarosh commented 1 year ago

@kungfooman transpiling mts/mjs in babel with a mixed esm/cjs setup is very quirky, and often rely on async babel usage, especially for @babel/register. TSC / esbuild / swc Π°re completely different beasts, in that regard, altogether.

jderochervlk commented 6 months ago

Hi! I know active development has moved to effect, but I'd be willing to tackle a setup for the repo to have dual CJS/ESM support. It would also solve this issue https://github.com/gcanti/fp-ts/issues/1910

I'll start working on a PR and hopefully have it ready by the end of the week.

gcanti commented 6 months ago

We've already tried it in the past, and it didn't go well. Shortly after the release, I received several open issues regarding setup problems, so I decided to revert the change and postpone the operation to the next major release.

jderochervlk commented 6 months ago

Will there be a major release for fp-ts? Or will these changes only apply to Effect?

joshderochervlk-simplisafe commented 2 weeks ago

Just want to follow up with this around cjs.

I'd be willing to put in the work to get an ESM/CJS version of fp-ts in a PR for a breaking v3, I just need to know if we want to do this and if it would be merged before I put in the effort.

mikearnaldi commented 2 weeks ago

A v3 of fp-ts is not currently planned, all the work moved to effect a while ago but we can't know if in the future there will be enough interest to wage a 3.0 and what the eventual scope would be. Re Effect we already ship dual modules and will most likely move to esim-only in the next major if require of esm modules become stable in node

joshderochervlk-simplisafe commented 2 weeks ago

I get that effect is the way forward, but for legacy projects already using fp-ts it would be helpful to have an ESM ready build. I would be willing to put the work in since this would be amazing for my team site to have.

mikearnaldi commented 2 weeks ago

I tend to agree to an extent I just mentioned that there is currently no plan, moving to esm for fp-ts would be a breaking, maybe the best way would be a fork which is esm only of the various fp-ts ecosystem projects

joshderochervlk-simplisafe commented 2 weeks ago

I could make a fork, but I don't know if that's the best option, especially if there is no plan for a v3 of the main lib.