Skip to content

Conversation

@joshwnj
Copy link
Member

@joshwnj joshwnj commented Jun 11, 2015

@joshgillies @rtsao please let me know if you think this should be approached differently, or have other ideas.

Notable changes:

  • --css plugin option is required (destination css file)
  • I've removed the .toString export, since css will be written to a file instead of being included in the js bundle (for consistency with the webpack implementation).

Once we've done some more testing on this we'll merge and then will need to upgrade https://github.com/css-modules/browserify-demo

@rtsao
Copy link
Contributor

rtsao commented Jun 11, 2015

Wow, that was fast! 👍

I think both o and output aliases would be nice for convenience with both CLI and API usage. I think it's pretty clear from the plugin that the output would be CSS (not to mention the file extension of the argument itself) so --css is probably fairly redundant.

I think omitting the .toString export by default makes sense, but IMO it might be good to leave it as an optional feature.

@joshgillies
Copy link
Member

Ping #9 #8

@joshwnj
Copy link
Member Author

joshwnj commented Jun 11, 2015

Good idea - going with convention of -o or --output

@joshwnj
Copy link
Member Author

joshwnj commented Jun 11, 2015

Will merge now, and we can pick up the remaining things after.

Regarding .toString I agree it might be useful at some point but let's wait until we have a real use case and model to that.

joshwnj added a commit that referenced this pull request Jun 11, 2015
@joshwnj joshwnj merged commit 9178c3b into master Jun 11, 2015
@joshwnj joshwnj deleted the plugin branch June 11, 2015 06:59
@joshgillies
Copy link
Member

👍

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.

4 participants