gigobyte / purify

Functional programming library for TypeScript - https://gigobyte.github.io/purify/
ISC License
1.5k stars 58 forks source link

esm version missing index.js #624

Closed atomictag closed 1 year ago

atomictag commented 1 year ago

I have just updated to 1.3.5 from 1.3.2 and started getting some weird errors in webpack. I think the root cause is the missing index.js/index.d.ts in the "esm" folder of the latest distribution (took me a while to figure this out - tooling error messages are cryptic to say the least). Oddly enough it was working fine in other setups - guess the "es" version was picked instead

matt-mcmahon commented 1 year ago

This change happened between v1.3.2 and v1.3.3. I'm guessing there was a breaking change in one of the dependencies? I don't see how any of the changes to purify source code or your package.json would have caused this.

You can work around the issue by using full import paths for the module your using; e.g. instead of import { Either } from "purify-ts" do import { Either } from "purify-ts/Either"

gigobyte commented 1 year ago

In v1.3.3 TypeScript was updated from 4.7.4 to 4.9.5. Looks like this issue is related: https://github.com/Zoltu/typescript-transformer-append-js-extension/issues/21

The problem is that ESM support was contributed by @semmel and I have no idea what the ESM spec is and how to generate output for it.

atomictag commented 1 year ago

The issue has definitely something to do with the Zoltu plugin (which essentially adds the .js extension to module imports as required by the esm spec). It is probably now incompatible with the latest typescript. From the top of my head solutions are:

The first approach is probably the best one and not a very risky option either. Beware that folks that are using this lib with bundlers which require ESM modules (like Rollup) are going to have big troubles with the most recent versions (my issue showed up with Webpack - and it took me ages to figure out what was breaking things). Maybe short term it would make sense to adjust the .ps script that copies things around to force all files to be copied were they belong (ultimately it's just a simple .ts and .d.ts pair of files)

connorjs commented 1 year ago

I have been curious about what FP library to try out in my latest side projects. I like Purify’s goals (respect that TS is not Haskell) and liked the “intro” vibe and nice documentation.

I did hit this ESM bug, but importing from purify-ts/Either seemed to allow me to patch over it (using esbuild).

I’m assuming the goal for this library is to double publish CJS and ESM? (Can someone confirm?)

connorjs commented 1 year ago

I took another look at the repo today and think this is the first question we should answer:

Why does this have three outputs?

Output Exports key Directory TS Config Related commits
Common JS require lib tsconfig.json (inferred) N/A
ES6 (ES2018) import lib/es tsconfig.es.json Commits
ESM module lib/esm tsconfig.esm.json Commits

Per my understanding, import is for ESM (Node wise) per https://nodejs.org/api/packages.html#conditional-exports. And we use "type": "module" in our (consuming) package.json to identify as ESM.

@semmel - Could you help clarify my understanding here? How do the import and module exports work together? Do you have any example ESM (TypeScript) projects consuming purify-ts (that depend on your change)? (I am using moduleResolution: nodenext in my TS project for Node app.)

gigobyte commented 1 year ago

I think we should drop the /es build because its main purpose was to produce smaller bundles for consumers with modern runtimes / browser requirements. Following that logic if native es support is not an issue I would assume esm support is not an issue as well, so we should consolidate /es into /esm

connorjs commented 1 year ago

I agree. It seems that we only want one of these two. I think the name esm makes more sense, too, yes.

So, I think the tentative recommendation is

  1. Remove es in favor of just esm
  2. Use import (not module) for the esm exports ← I think, but would like confirmation
gigobyte commented 1 year ago

I'll get to it this weekend, in the meantime any help/PRs are welcome.

gigobyte commented 1 year ago

@connorjs @atomictag @matt-mcmahon Can any of you confirm that the new 2.0.0 version is working fine?

matt-mcmahon commented 1 year ago

v2.0.0 looks good 👍

The index.js file is where TS/Node will expect it. Current version of my project builds with Vite and Purify v2.0.0 successfully, no errors in my use case due to removal of the ES folder.

My project isn't currently broken under 1.3.5, so I checked out an earlier release that I knew broke after upgrading from 1.3.2 to 1.3.3.

  1. While purify 1.3.2 is installed, Vite build works, no error.
  2. Upgraded purify to 1.3.3 and the Vite build failed with the incorrect module specifier error.
  3. Upgraded purify to 2.0.0 and the build was okay again, no other changes to my project.

So TS/Node seems to be finding the index.js file where it expects it. I mean, it should because the file's exists again, but there's enough complications with how ESM imports work that I wanted to double check. 😅

connorjs commented 1 year ago

v2.0.0 looks good to me too. I updated and was able to change the "purify-ts/Either" imports to just "purify-ts".

gigobyte commented 1 year ago

Amazing news!