-
Notifications
You must be signed in to change notification settings - Fork 360
feat: add --pkg-main option #658
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
feat: add --pkg-main option #658
Conversation
|
Nice - @wardpeet and I were looking at breaking those functions out anyway, and I like the way you've done it here! |
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.
Stellar work, as usual.
| const { pkg } = options; | ||
| const pkgMain = options['pkg-main']; | ||
|
|
||
| if (!pkgMain) { |
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 wonder if we could even apply this same logic if the value provided for the --output argument has a file extension. Like microbundle -f umd -o bundle.js would see .js and know to use that name as-is.
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 think this would open to questions:
- Such a new logic could be a "breaking" change because there might be people already using -o with ".js" and expecting it to add the format suffix. If a breaking change would mean that this has to wait for 1.0 I personally would prefer to split this idea into another issue 😇
- If we implement this logic, it should only ever apply if the format option is set with a single value
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.
Agree with @katywings here. I'm worried that us making too many assumptions will lead to unexpected results for the user. In either case this would be a breaking change.
5378311 to
859fdba
Compare
marvinhagemeister
left a comment
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.
Awesome, thank you so much for the PR 🙌
The goal of this feature is to add a solution for microbundle, to manually disable the package.json / format suffix automations analog to https://twitter.com/Katy_Wings/status/1271189844077748224
The new option
--pkg-mainby default is true - basically notifying the consumer that main entries in package.json will be considered. The consumer can supply--pkg-main falseto disable the microbundle package.json "sideffects".This PR also includes a little refactor of the "main" generation - most of it is the same as before but just moved into separate functions.