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

Request: update docs for recommended programmatic usage post-ESM #888

Closed
zackdotcomputer opened this issue Mar 23, 2022 · 12 comments
Closed
Labels
enhancement New feature or request good first issue Straightforward problem, solvable for first-time contributors without deep knowledge of the project PRs welcome PRs are welcome to solve this issue!

Comments

@zackdotcomputer
Copy link

Right now the README explains how to import and use the openapiTS function for programmatic usage, but doesn't give context around how to do so for folks who are on LTS node or ts-node with default settings (i.e. did not have "type": "module" in their package.json and are just trying to run an importTypes.ts script with ts-node).

I think that ESM is just now becoming the default but the tooling around it is still pretty early and confusing between the prompts to add "type": "module" fighting with ts-node suddenly forgetting how to read TS files when you do. It would be nice to not only provide guidance on what should go in one's importTypes script but also the recommended way to run it now that the 5.x releases are shipping without a CJS copy.

@drwpow drwpow added enhancement New feature or request PRs welcome PRs are welcome to solve this issue! good first issue Straightforward problem, solvable for first-time contributors without deep knowledge of the project labels Mar 23, 2022
@drwpow
Copy link
Contributor

drwpow commented Mar 28, 2022

I’d be in favor of adding this. It might be easier if I wrote the docs, but I’d love to source info from people:

  1. What are you using to compile TypeScript (tsc? ts-node? babel? esbuild?)
  2. Are you using a bundler (webpack, vite, etc.)
  3. Are you transforming your TypeScript with something other than the TS compiler (e.g. Babel)?
  4. What platform are you targeting? (Node version, along with whether a CJS or ESM build)
  5. What is the final code that gets built/run (e.g. does your code transform to require('openapi-typescript') or import … from 'openapi-typescript')?
  6. Most important what were you able to do to resolve your issue? (it’s OK if you weren’t, but please provide info if you did)

There won’t be a one-size-fits-all recommendation to people because all of these individual choices likely affect the fix. But with enough info we can probably have enough recommendations for most people to find something that works. There are just many factors!

Unfortunately, ESM errors are all a side-product of a runtime completely replacing its module system, and it’s messy and frustrating for all involved (no shade at all to the Node team; I think they’re doing a wonderful job! It’s just an incredibly-hard problem dealing with upgrading the largest open software repository in history)

@jonasgroendahl
Copy link

would be great

@zackdotcomputer
Copy link
Author

@drwpow I can answer your questions but with the caveat that my case is a little bit of a special case. For us, we're using this project in a JS script file that runs on-build to generate an API types file. We could npx this except we are passing in a custom formatter function to perform transforms such as uuid to string.

So the answers to your questions for our use of this project:

  1. I think this is irrelevant - the import here is in a plain .js file that we run with node. We're using NextJS's Rust compiler for the final build, though.
  2. Not when calling this package because this is used in a script.
  3. Same answer as 2
  4. CJS and Node 14.
  5. The exact line is const openapiTS = require("openapi-typescript").default;
  6. I downgraded back to 4.5.0 because it was working relatively fine and I didn't have time to dedicate to unwinding how to fix this.

I guess this might be off topic but I'm curious why this package has to ship with only ESM rather than with both ESM and CJS side-by-side? I use tsdx for my OSS libraries and I know it supports the ability to ship packages that have both structures exposed?

@quaos
Copy link

quaos commented Jan 8, 2023

I'm running into this when trying to import the package from within gulpfile.js. Seems this use case is currently not supported?

(I can create a TS script file within my main project which is already TS. But that complicates things, so I prefer to generate OpenAPI TS files before transpiling source files in the project.)

@ianwremmel
Copy link

I spent a few hours fighting with this today and came up with my own answers to your questions.

  1. I'm using at least ts-node, swc, babel, and possibly esbuild. The state of tooling various tools auto-included runners subject me to many different tools that I have limited control over.
  2. Not in situations where I'm using openapi-typescript.
  3. See 1
  4. Node LTS. See 1 - I'm trying to write a plugin for nx which, amongst other things, compiles openapi to typescript. From what I can tell, their defaults (via ts-node or swc) are compiling to CJS on the fly, prior to the import of openapi-typescript.
  5. See 4
  6. Not so far. Will likely either downgrade or use execSync to shell out to the cli (which seems goofy, but is probably the easiest option).

To second what @zackdotcomputer said: the answers to these questions don't really matter. The state of ESM is pretty terrible and, to quote a similar issue in a completely different project, "esm-only dependencies are user-hostile". For this project in particular, most of the use-cases is probably going to end up being it-looks-like-esm-but-it-runs-like-cjs-on-the-fly-compilation, so I'd strongly request publishing a ESM and CJS builds as part of your release process.

@drwpow
Copy link
Contributor

drwpow commented Jul 31, 2023

@ianwremmel I have only dabbled with nx for a bit. I don’t know if you could use vite-node as a replacement for ts-node, but I guarantee it would work with openapi-typescript (it powers the test suite). I also can’t recommended it enough as a general universal runner, but I’m not sure if nx would give you that option.

@drwpow
Copy link
Contributor

drwpow commented Jul 31, 2023

Also for a little more context, this library was originally shipped as CJS several years ago, but for a brief period did ship CJS + ESM, but dropped that because both users were having problems (especially when you throw TypeScript in the mix). Believe it or not, it’s not a simple “just use X to build” solution (I actually have experience with this, having worked on Snowpack which was the precursor to Vite). Any libraries that have had good CJS + ESM support (which are rare—many libraries still just ship good quality CJS and broken ESM) have earned it the hard way by fixing dozens if not hundreds of issues. But there’s always another “doesn’t work with X” bug lying in wait.

I’m open to taking a second look at shipping a companion CJS build again; it was just dropped at a time when I knew I didn’t have the resources to maintain both shipped bundles and the core library, but also had more dependencies (like Prettier) that made bundling for multiple targets tricky. Now that this library is virtually dep-free I think it should be a much easier support story, and one we could tackle.

@ianwremmel
Copy link

First off, let me apologize for coming off harshly yesterday. The state of ESM is really frustrating and, worse, most of the projects that have moved to ESM-only seem to be the ones that are used in scenarios where it's least helpful (e.g., ESM is mostly only valuable for things that need to be tree-shaken when bundle size really matters) and hardest to work around (e.g., stuff that'll end up in CLIs).

Without a lot of mucking with internals, many CLIs have their own built-in ways of on-the-fly compilation that are hard or impossible to avoid. The following come to mind, but I've encountered others:

  • jest pretty much requires you to use babel-jest and therefore babel for typescript support (I've tried other options and I've never quite gotten them work)
  • nx uses ts-node (or swc if it's installed) and it does things that interrupt any require hooks you might try to use (e.g., even if nx were run with vite-node, it runs most stuff via child_process, so those hooks are lost)
  • storybook uses webpack and therefore babel. There's an option for vite, but it doesn't work quite the same, so making that migration requires changing a bunch of implementation code.

All that being said, I've been shipping CJS, ESM, and types pretty successfully in this monorepo. Admittedly, I'm the only consumer 😂.

The build commands are as simple as

esbuild $FILES --format=cjs --outdir=$DIST_DIR/cjs --platform=node --sourcemap=external

and

esbuild $FILES --format=esm --outdir=$DIST_DIR/esm --platform=node --sourcemap=external

and

tsc --project path/to/tsconfig.json #tsconfig should list $DIST_DIR/types as its output directory

and then package.json contains an export map:

{
  "exports": {
    ".": {
      "import": "./dist/esm/index.js",
      "require": "./dist/cjs/index.js",
      "types": "./dist/types"
    },
    "./package.json": "./package.json"
  },
}

With this setup, it's important to remove "module:" <file> (and possibly the "type:" "module") entries from package.json

note that the types entry is different from what you'll see in the repo. I just discovered last night that TypeScript changed how types are resolved when moduleResolution is set to node16 and I haven't updated the repo yet.

@drwpow
Copy link
Contributor

drwpow commented Jul 31, 2023

No need to apologize—you didn’t come across harshly at all! 🙂 I feel your frustration, and I have invested a lot of my life in the CJS <> ESM compatibility interop outside of this project. I completely understand where you’re coming from!

A bit irrelevant, but what you described is the exact reason why I moved away from Jest > Vitest; webpack > Vite; etc etc personally. Storybook has gotten half-Vite support, but it has also been a major source of frustration because it is still hard-tied to webpack in areas. But that said, I know that what works for me may not be possible for others, and it’s wrong to assume a solution to a problem is “just replace X with Y.”

Anyways, I do believe it is this library’s responsibility to make it easy to consume. But I still believe in an ESM-first ecosystem as a universal module standard for Node & web, with CJS being a tack-on to support legacy systems. But that belief, again, is unhelpful to users experiencing pain, to all your points. And as a person who has had a lot of pain in that area myself, I empathize and am not writing it off.

@drwpow
Copy link
Contributor

drwpow commented Jul 31, 2023

I just discovered last night that TypeScript changed how types are resolved when moduleResolution is set to node16 and I haven't updated the repo yet.

And yes—this is a great example of one of the many problems that make shipping multi-target builds hard for CJS + ESM with full TypeScript support for both 🙃

@drwpow drwpow mentioned this issue Jul 31, 2023
3 tasks
@drwpow
Copy link
Contributor

drwpow commented Jul 31, 2023

Update: published another attempt at an ESM + CJS bundle at 6.4.0. If people could give that a try and let me know if it’s working for them (or not), I’d appreciate it 🙏. Though I can test basic examples, the devil’s in the details and people in this issue probably have specific setups that require some tweaks I’m not aware of.

Also, thanks for everyone speaking on this issue. It was worth looking into again, because several barriers to publishing a good CJS package were removed from the last attempt.

@ianwremmel
Copy link

Looks like it works, thanks!

@drwpow drwpow closed this as completed Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Straightforward problem, solvable for first-time contributors without deep knowledge of the project PRs welcome PRs are welcome to solve this issue!
Projects
None yet
Development

No branches or pull requests

5 participants