-
-
Notifications
You must be signed in to change notification settings - Fork 503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop CJS #1777
Comments
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 |
I got bundler-free ESM to work from It's a pity that TypeScript can't just add Used regular expressions:
|
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 |
I don't think the build process should touch imports, we have really 2 choices here:
There was a long discussion in TS about adding |
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 |
@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. |
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. |
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. |
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) |
What do you mean by 'ts allows js'? |
You can use @mikearnaldi You did not get it; that script does not rewrite anything; it's only a check. |
@mikearnaldi microsoft/TypeScript#16577 (comment) ".js file extensions are now allowed" |
@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 |
Meanwhile, I tested two eslint plugins for checking file extensions. Guess what, no one works. This one check also packages, can't be disabled. This does not work either. |
Funny thing fp-ts@2.12.3 with Node 16 ESM: I need
For some reason, importing from lib works 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.
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 In effect to have both working I use:
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 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 |
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) |
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. |
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. |
I will recommend still building to There seems to be better support for extensionless imports with I don't know if this is just a |
I'm using
My version of |
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
<#1777 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFZAKCX6DSV754HPCKX5SPLWGL5LHANCNFSM6AAAAAAQN7KVYQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@mikearnaldi It works! Thanks! Off topic: Why are all the useful things in |
You're welcome! So the idea is to separate a bit the implementations to keep it more maintainable and focused, in
You can check the repositories in the orgs, in the one for |
Yeah the Unfortunately using |
I'd say that I'm still dual-building projects with a mix of Having a |
You don't even need 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. 😂 |
@kungfooman transpiling |
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 #1910 I'll start working on a PR and hopefully have it ready by the end of the week. |
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. |
Will there be a major release for |
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. |
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 |
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. |
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 |
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. |
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).
The text was updated successfully, but these errors were encountered: