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.

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

Add target environment option #38

Merged
merged 3 commits into from
Jan 13, 2018
Merged

Add target environment option #38

merged 3 commits into from
Jan 13, 2018

Conversation

kevlened
Copy link
Contributor

@kevlened kevlened commented Jan 7, 2018

When building isomorphic libraries, it's useful to bundle browser versions of dependencies. The target option makes this possible and provides flexibility to make environment-based decisions in the future.

@@ -13,6 +13,7 @@ prog
.option('--entry, -i', 'Entry module(s)')
.option('--output, -o', 'Directory to place build files into')
.option('--format, -f', 'Only build specified formats', 'es,cjs,umd')
.option('--target', 'Specify your target environment', 'node')
Copy link
Contributor Author

@kevlened kevlened Jan 8, 2018

Choose a reason for hiding this comment

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

I use target, because that's the name in webpack. However, environment or platform may be more descriptive.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm good with replicating webpack's naming there.

src/index.js Outdated
@@ -206,7 +206,8 @@ function createConfig(options, entry, format) {
}),
useNodeResolve && nodeResolve({
module: true,
jsnext: true
jsnext: true,
browser: options.target === 'browser'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we just avoid the extra option and set browser: true always? If you're building for node, you'll use --external all anyway, so this will never be hit.

Copy link
Owner

Choose a reason for hiding this comment

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

I'd just do browser: options.target!=='node'.

@developit developit merged commit e9683e1 into developit:master Jan 13, 2018
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.

2 participants