-
Notifications
You must be signed in to change notification settings - Fork 193
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
Added back CommonsJS output under /cjs #1956
Conversation
The main ts-json-schema-generator.ts file imported https://github.com/vega/ts-json-schema-generator/actions/runs/9105923115/job/25032310451#step:7:997 |
ts-json-schema-generator.ts
Outdated
import { mkdirSync, writeFileSync } from "fs"; | ||
|
||
// This constant gets replaced by the build script | ||
const pkgVersion = "0.0.0"; /* __VERSION__ */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this as it makes it harder to watch build. The cjs version used to work so can you try to make the import version work here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a lot but ran out of choices.
-
import pkg from 'package.json'
(or * as pkg) works in cjs but not in ESM as it requires awith {type: 'json'}
statement -
require('package.json')
works but not in ESM mode. -
using
const _require = typeof require === 'undefined' ? createRequire(import.meta.url) : require
should work but breaks in CJS even ifcreateRequire(import.meta.url)
does not gets executed. -
using
fs.readFileSync
requires__dirname
(which can be created usingimport.meta.url
in esm but breaks just like the above try) and a second build step to manually copy package.json todist
/cjs
folders. -
Trying to bypass
import.meta.url
using `eval('import.meta.url') does not work.
I'm not sure if there's any other way to fix it without replacing the dist code. Also, as it gets built on CI before publishing, all official releases should work just as fine.
After all, the only thing I can say is: Welcome to the hell of maintaining ESM/CJS modularity in modern JavaScript. |
True. I've wasted too many days with bundling and packaging stuff. |
is there anything left? |
replaced by #1964 |
#1922 (comment) removed support for CommonsJS entirely without warnings and a major release. Since a lot of libraries depends on this package, adding a
cjs
bundle back should be considered.You may argue that their libraries should migrate to ESM, however, one of the uses cases of this library is to generate json schemas inside a Typescript Plugin which is loaded by the TypeScript CJS source itself and will not be ported to ESM soon (if ever).