Skip to content

Conversation

@katywings
Copy link
Collaborator

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-main by default is true - basically notifying the consumer that main entries in package.json will be considered. The consumer can supply --pkg-main false to 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.

@developit
Copy link
Owner

Nice - @wardpeet and I were looking at breaking those functions out anyway, and I like the way you've done it here!

Copy link
Owner

@developit developit left a 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) {
Copy link
Owner

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.

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 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

Copy link
Collaborator

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.

@katywings katywings force-pushed the feature-no-format-suffix branch from 5378311 to 859fdba Compare June 12, 2020 19:07
Copy link
Collaborator

@marvinhagemeister marvinhagemeister left a 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 🙌

@marvinhagemeister marvinhagemeister merged commit f4ab33d into developit:master Jun 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants