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

Bundle with Rollup #96

Merged
merged 6 commits into from Oct 15, 2016
Merged

Bundle with Rollup #96

merged 6 commits into from Oct 15, 2016

Conversation

Rich-Harris
Copy link
Contributor

As discussed in #87. This change only affects people who are using the dist files (their app will get smaller and boot faster) and people using Webpack 2 or Rollup (allows them to consume the library as an ES module).

const prod = process.env.PRODUCTION
const mode = prod ? 'production' : 'development'

console.error(`creating ${mode} bundle...`)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this a console.error? (can we also capitalize the first c?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just because that prints to stderr (i.e. metadata) rather than stdout (i.e. data). No actual difference, just force of habit! Can change to console.log (or remove altogether) if you prefer

Copy link
Member

Choose a reason for hiding this comment

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

It makes me feel weird to console.error on every build, I'd prefer it to be console.log! 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

Can we have some before/after stats? (or are they the same as in the original comment, i.e. 13% smaller UMD build and 60% quicker parse?)

if (id === processShim) return 'export default { argv: [], env: {} }'
return null
},
},
Copy link
Member

@mxstbr mxstbr Oct 15, 2016

Choose a reason for hiding this comment

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

Can we get some comments in the file what this whole block is needed for? We'll need to maintain this, so not having any idea what this is there for is a big unfortunate!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Rich-Harris
Copy link
Contributor Author

Can we have some before/after stats? (or are they the same as in the original comment, i.e. 13% smaller UMD build and 60% quicker parse?)

Yep, nothing has changed from earlier

Copy link
Member

@mxstbr mxstbr 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!

@mxstbr mxstbr merged commit 0dd5bfe into styled-components:master Oct 15, 2016
@Rich-Harris Rich-Harris deleted the rollup branch October 15, 2016 18:40
@mxstbr mxstbr mentioned this pull request Oct 16, 2016
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.

None yet

2 participants