feat: added support for cjs and esm#793
Conversation
erunion
left a comment
There was a problem hiding this comment.
oh wow! thanks so much for this. i'm going to wait for @kanadgupta to take a look at it before merging it in.
kanadgupta
left a comment
There was a problem hiding this comment.
Hey thanks for this @prisis! I did some preliminary investigation and noticed a few issues below. Happy to jump in and help this get across the finish line when I'm back in office next week.
There was a problem hiding this comment.
it looks like this file got converted to lockfileVersion 1, mind bumping it to v3? if you're using npm v8 or higher you should be able to run this (source):
npm i --lockfile-version 3There was a problem hiding this comment.
Sorry, had a old version of npm (using most of the time pnpm :D)
| "description": "Comprehensive tooling for working with OpenAPI definitions", | ||
| "license": "MIT", | ||
| "author": "ReadMe <support@readme.io> (https://readme.com)", | ||
| "sideEffects": false, |
There was a problem hiding this comment.
This is used for https://webpack.js.org/guides/tree-shaking/
| "require": "./dist/index.js", | ||
| "import": "./dist/index.mjs" | ||
| }, | ||
| "./package.json": "./package.json" |
There was a problem hiding this comment.
A lot of packages are you this export for example preact or nextjs
This can be used to support more then one version of a package
import pkg from "oas/package.json"
pkg.version....
tsup.config.ts
Outdated
| clean: true, | ||
| splitting: true, | ||
| shims: true, | ||
| target: tsconfig.compilerOptions.target as 'es2020', |
There was a problem hiding this comment.
don't think this is needed. per their docs:
The value for
targetdefaults tocompilerOptions.targetin yourtsconfig.json
| target: tsconfig.compilerOptions.target as 'es2020', |
tsup.config.ts
Outdated
|
|
||
| export default defineConfig((options: Options) => ({ | ||
| ...options, | ||
| entry: ['src/index.ts'], |
There was a problem hiding this comment.
not your fault here since we don't really document but this well, but we rely on a few individual non-exported files from this library over in rdme (example). i tried checking out this branch locally and npm link-ing but wasn't able to get this working (even with the changes below) so this will require some deeper investigation 😕 is there a way to get tsup to compile without bundling?
| entry: ['src/index.ts'], | |
| entry: ['src/index.ts', 'src/rmoas.types.ts', 'src/analyzer/index.ts', 'src/lib/reducer.ts'], |
There was a problem hiding this comment.
Will add the operation.ts to the list too, saw it someware
|
Love this! I was just trying out the Bun test runner and noticed a bug(probably in Bun itself). This is of course fine but then I was wondering with the change done here we only can import This will reduce the size of the package(duplicated code) and make sure that we import from |
|
@stijnvanhulle Sure, want to submit a PR? |
🧰 Changes
🧬 QA & Testing
Check the output for cjs and esm