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

fix: Use css sourceMaps in development #591

Closed
wants to merge 1 commit into from
Closed

fix: Use css sourceMaps in development #591

wants to merge 1 commit into from

Conversation

JaKXz
Copy link

@JaKXz JaKXz commented Sep 5, 2016

Fixes #590.

@gaearon this is a simple solution - and thanks to the comments in this file I don't think I'm missing anything? :)

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost
Copy link

ghost commented Sep 5, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ghost ghost added the CLA Signed label Sep 5, 2016
@gaearon gaearon added this to the 0.4.2 milestone Sep 6, 2016
@ghost ghost added the CLA Signed label Sep 6, 2016
@mareksuscak
Copy link
Contributor

Could you please configure publicPathas per the css-loader's docs to ensure source maps work correctly?

@JaKXz
Copy link
Author

JaKXz commented Sep 13, 2016

@mareksuscak I'm honestly not sure what you mean. I've never used publicPath and haven't had any issues with css-loader's sourceMaps. I also don't see a documented example.

I did a quick search through the css-loader codebase and couldn't find any references to publicPath either. Could you help me out?

@ghost ghost added the CLA Signed label Sep 13, 2016
@mareksuscak
Copy link
Contributor

Hm, how come? My link is pointing directly to the subsection where it's mentioned. See the attached screenshot with a red rectangle that highlights the particular subsection.

source-maps

It is also noted in the SurviveJS guide. See the pic below.

screen shot 2016-09-13 at 09 57 07

Not sure if that has or hasn't been fixed.

@JaKXz
Copy link
Author

JaKXz commented Sep 13, 2016

I see where it's mentioned - I've just never seen an example in the wild
(due to lack of necessity). But sure I'll take a look, and you can check
out this branch and verify it too?
On Tue, Sep 13, 2016 at 03:58 Marek Suscak notifications@github.com wrote:

Hm, how come? My link is pointing directly to the subsection where it's
mentioned. See the attached screenshot with a red rectangle that highlights
the particular subsection.

[image: source-maps]
https://cloud.githubusercontent.com/assets/613647/18465687/45d0223c-7998-11e6-9d9d-6a4b9346837d.png

It is also noted in the SurviveJS guide
http://survivejs.com/webpack/developing-with-webpack/enabling-sourcemaps/#sourcemaps-for-styling.
See the pic below.

[image: screen shot 2016-09-13 at 09 57 07]
https://cloud.githubusercontent.com/assets/613647/18465721/79dde5f0-7998-11e6-9248-29bd17167335.png

Not sure if that has or hasn't been fixed.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#591 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AChScZ1VzfdAyP7R7AstR1nomQUifv7Gks5qplergaJpZM4J1TDd
.

@mareksuscak
Copy link
Contributor

Sure, will do when I get a chance today.

@gaearon
Copy link
Contributor

gaearon commented Sep 16, 2016

As noted in Contributing.md, please include a test plan for this pull request. I don’t know how to verify that it works if you don’t tell me. 😉

@gaearon
Copy link
Contributor

gaearon commented Sep 17, 2016

I get a flash of unstyled content on Chrome 52 with this feature enabled.

wat

I’m not sure if it’s a css-loader, browser bug, or something else, but it’s annoying and will confuse our users. Could you look into why this happens?

Another area I’d like to see addressed is the performance impact. Can you create ~200 CSS files and measure first build and rebuild performance in development with and without this change?

Copy link
Contributor

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

Need to solve flash of unstyled content, and provide build performance measurements.

@gaearon gaearon removed this from the 0.4.2 milestone Sep 17, 2016
@ghost ghost added the CLA Signed label Sep 17, 2016
@JaKXz
Copy link
Author

JaKXz commented Sep 27, 2016

Apologies for the lack of response on this - I'm not really sure where to start looking for the problem here though my guess would be to start in css-loader. I don't have the OSS cycles I'd like to dedicate to this, unfortunately, so someone can feel free to take or close this.

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2016

Let’s close then as the implications are unclear.
Happy to consider a similar PR if somebody figures out what’s wrong.
cc @TheLarkInn

@gaearon gaearon closed this Sep 27, 2016
@TheLarkInn
Copy link

FOUC from style source-maps? And chrome 52 (just making sure I'm catching everything I'm reading here).

Would be curious to see where those scripts are dropped on the page when rendered.

@mareksuscak
Copy link
Contributor

mareksuscak commented Sep 28, 2016

I had some time today and here's what I've found. When you use ?sourceMap option with css-loader, instead of using style tags with inline styles, webpack injects style tags that reference the dynamically generated files (source maps likely don't work well with the inline style tags). Hence we need to load a few more files after we've already run the JavaScript part which obviously causes FOUC as the HTML has already been appended. Not sure if there's a cure for that. Production env should not be affected at all.

I remember este.js suffered from FOUC in development env too in the past. Now they use JS styles.

screen shot 2016-09-28 at 04 00 33

cc @gaearon @TheLarkInn @JaKXz

@gaearon
Copy link
Contributor

gaearon commented Sep 28, 2016

Wow, thanks for looking into it. That makes sense now.

@TheLarkInn
Copy link

Thank you. Just my 2 cents, I'd say the dev env FOUC is worthwhile cost for extra source map features (which are handy). Especially if not affected in production (which makes sense now also).

@gaearon
Copy link
Contributor

gaearon commented Sep 28, 2016

I'm worried that not only FOUC will be annoying, people will think it's something they did wrong, or worse, that it's React problem. These days people blame everything on React so we don't want to make it easier 😉

@TheLarkInn
Copy link

Lol good point!

@skaltenegger
Copy link

skaltenegger commented Apr 20, 2017

I got CSS sourcmaps and hot reloading working by following @mareksuscak's advice of changing my css loader in the webpack config to the following:

{ test: /\.css$/, loader: 'style!css?sourceMap!postcss' },

And I can't complain about FOUC (flash of unstyled content) - so all good for development!

Hope tat may help someone here! 😎

@zikaari
Copy link
Contributor

zikaari commented Sep 27, 2017

@gaearon

Need to solve flash of unstyled content, and provide build performance measurements.

Done #3202 - Add support for CSS Sourcemaps without FOUC

@hugomallet
Copy link

Hi,

Any update about css / sass sourcemaps in development mode (npm start) ?

@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants