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

react-scripts@2 final blockers #5141

Closed
19 of 25 tasks
gaearon opened this issue Sep 27, 2018 · 13 comments
Closed
19 of 25 tasks

react-scripts@2 final blockers #5141

gaearon opened this issue Sep 27, 2018 · 13 comments
Milestone

Comments

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2018

Let's just keep track in one list so we don't forget.

Code Blockers

Docs Blockers

  • Remove anything outdated from the User Guide
  • Make a decent changelog and migration guide from 1.x

Docs Follow-ups

@bugzpodder
Copy link

should we remove integration for #4563

@gaearon
Copy link
Contributor Author

gaearon commented Sep 27, 2018

updated

@Timer Timer added this to the 2.0.0 milestone Sep 27, 2018
@iansu
Copy link
Contributor

iansu commented Sep 27, 2018

It sounds like #5140 has been resolved and is not a bug.

@Timer
Copy link
Contributor

Timer commented Sep 27, 2018

Yeah, but it's still crappy. Can we disable that somehow? There's no such thing as "unused" CSS when its imported.

@iansu
Copy link
Contributor

iansu commented Sep 27, 2018

I guess that's a webpack question. I think that's just part of their tree-shaking implementation though.

Technically this is a bug in the third-party package. They've declared that their package is side effect free when it's really not.

@Timer
Copy link
Contributor

Timer commented Sep 27, 2018

Sure, but css should be an implied sideEffect, despite what the package.json says.

@edmorley
Copy link

edmorley commented Sep 28, 2018

Can we disable that somehow? There's no such thing as "unused" CSS when its imported.

webpack/webpack#6571 would help with this.

The only other workarounds would be to:

  1. set optimization.sideEffects to false (but that will increase bundle size)
  2. see if it's possible to categorise all CSS as having side-effects by setting Rule.sideEffects for the css-loader entry in rules (but it's not clear which of that or the package.json entry takes priority - would need testing)

Edit:
It looks like (2) would work, since it's applied after the package.json value so can override it:
https://github.com/webpack/webpack/blob/8a7597aa6eb2eef66a8f9db3a0c49bcb96022a94/lib/optimize/SideEffectsFlagPlugin.js#L24-L49

@gaearon
Copy link
Contributor Author

gaearon commented Sep 28, 2018

I'm okay with switching to this behavior as long as migration notes mention it as potential pitfall and explain what to tell library authors.

@Timer
Copy link
Contributor

Timer commented Sep 28, 2018

@edmorley can you send a PR for this, please?

@gaearon
Copy link
Contributor Author

gaearon commented Sep 28, 2018

see if it's possible to categorise all CSS as having side-effects by setting Rule.sideEffects for the css-loader entry in rules (but it's not clear which of that or the package.json entry takes priority - would need testing)

This is what webpack should do by default IMO. There's no way for CSS file to "know" if it's effectful. Only the loader can tell that since loader determines how to interpret it. Side-effectfulness of style-loader should've "marked" the CSS file somehow.

@eward957
Copy link

I really hope react-scripts@2 can be supports "less" & "ts" directly 💯 👍

@gaearon
Copy link
Contributor Author

gaearon commented Sep 29, 2018

Added a few things here.

@Timer Timer mentioned this issue Oct 1, 2018
7 tasks
@Timer
Copy link
Contributor

Timer commented Oct 1, 2018

Dropped remaining items in #5190, tagged with correct milestone.

@Timer Timer closed this as completed Oct 1, 2018
@lock lock bot locked and limited conversation to collaborators Jan 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants