fix: remove createRequire for package json #978
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
| banner: { | ||
| js: `import { createRequire } from 'module'; const require = createRequire(import.meta.url);`, |
There was a problem hiding this comment.
You removed createRequire, but Is it needed?
There was a problem hiding this comment.
prompts dynamically requires readline causing this error when the banner is omitted:
Error: Dynamic require of "readline" is not supported
There was a problem hiding this comment.
We could look into moving to inquirer.js from prompts:
https://github.com/SBoudrias/Inquirer.js
It seems like it is more actively maintained, so I'd imagine the esm support is more up to date and we could then remove this banner.
There was a problem hiding this comment.
Previously, I rejected the idea #339 by @himself65 because I thought it was too early.
But now that we have more active contributors, I think the time is right.
I don't think prompts is too bad, but if you guys have a better alternative, and feel like maintaining it, I'm fine of changing the dependency library, or completely re-implementing it.
| format: ['esm'], | ||
| minify: !options.watch, | ||
| clean: true, |
| // keep original require to avoid | ||
| // bundling the whole package.json by `@vercel/ncc` | ||
| const packageJson = createRequire(import.meta.url)('../package.json'); | ||
| const packageJson = await import('../package.json'); |
There was a problem hiding this comment.
So, is this syntax supported by tsup?
follow up to this: #974 (comment)