Skip to content
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

esm version missing index.js #624

Closed
atomictag opened this issue Mar 2, 2023 · 12 comments
Closed

esm version missing index.js #624

atomictag opened this issue Mar 2, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@atomictag
Copy link

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
Copy link

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
Copy link
Owner

gigobyte commented Mar 3, 2023

In v1.3.3 TypeScript was updated from 4.7.4 to 4.9.5. Looks like this issue is related: Zoltu/typescript-transformer-append-js-extension#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
Copy link
Author

atomictag commented Mar 3, 2023

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:

  • remove the Zoltu plugin stuff and add .js extension to all modules/files (this is compatible with cjs stuff as well so the only downside is changing each and every file)
  • revert to an earlier version of typescript which is still compatible with that plugin. Given the heavy reliance on TS in this project this solution seems not scalable in the long term

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)

@gigobyte gigobyte added the bug Something isn't working label Mar 4, 2023
@connorjs
Copy link

connorjs commented Mar 5, 2023

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
Copy link

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
Copy link
Owner

gigobyte commented Mar 16, 2023

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
Copy link

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
Copy link
Owner

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

@gigobyte
Copy link
Owner

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

@matt-mcmahon
Copy link

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
Copy link

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
Copy link
Owner

Amazing news!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants