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.

Sign up for GitHub

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

Jump to bottom

fix(build): disable esModule and re-export for nodejs #1486

Merged
merged 1 commit into from Jan 1, 2023
Merged

fix(build): disable esModule and re-export for nodejs #1486

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion rollup.config.js
View file
Open in desktop
Expand Up @@ -81,8 +81,9 @@ function createCommonJSConfig(input, output, options) {
output: {
file: `${output}.js`,
format: 'cjs',
esModule: false,
outro: options.addModuleExport
? 'module.exports = exports.default; Object.assign(exports.default, exports);'
? ';(module.exports = (exports && exports.default) || {}),\n Object.assign(module.exports, exports)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why ,\n instead of ;? Mostly out of curiosity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it in a minified bundle locally and copied it from there so that’s the minified version

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced that ,\n is more minified than ;, but don't care much..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I didn't do it 😂, it was from the minifier of the I was trying all this with and most of them do that
Screenshot 2022-12-17 at 5 52 29 PM

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's actually minified. but adding ;(...) increased 3 bytes.

What I would do is something like:

Suggested change
? ';(module.exports = (exports && exports.default) || {}),\n Object.assign(module.exports, exports)'
? ';module.exports=exports&&exports.default||{};Object.assign(module.exports,exports)'

Hmm, we can actually combine it?

Suggested change
? ';(module.exports = (exports && exports.default) || {}),\n Object.assign(module.exports, exports)'
? ';Object.assign((module.exports=exports&&exports.default||{}),exports)'

(I'm just enjoying the puzzle. Whatever works is okay to merge.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll need another addition of , cause the return of the above wouldn't be either exports or an empty object and not actually assign stuff to exports so
Object.assign(((module.exports = (exports && exports.default) || {}), module.exports),exports);

Would be the working one and compared to that the original commit code is smaller 😂

: '',
},
external,
Expand Down