perf: generate source maps only when explicitly set #478

alexander-akait · 2017-04-08T15:14:11Z

What kind of change does this PR introduce?
bugfix

Did you add tests for your changes?

not required

If relevant, did you update the README?

not required

Summary

webpack-contrib/sass-loader#296 (comment)

Does this PR introduce a breaking change?

No

codecov · 2017-04-08T15:15:43Z

Codecov Report

Merging #478 into master will decrease coverage by 0.25%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master     #478      +/-   ##
==========================================
- Coverage    98.6%   98.35%   -0.26%     
==========================================
  Files          10       10              
  Lines         359      364       +5     
  Branches       81       85       +4     
==========================================
+ Hits          354      358       +4     
- Misses          5        6       +1

Impacted Files	Coverage Δ
lib/processCss.js	`98.01% <100%> (+0.02%)`	⬆️
lib/loader.js	`98.61% <80%> (-1.39%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b90d8dd...7c1cb0a. Read the comment docs.

michael-ciniawsky

@evilebottnawi 👍 Please add test(s) for this

alexander-akait · 2017-04-11T09:06:43Z

/cc @michael-ciniawsky @bebraw @d3viant0ne

bebraw · 2017-04-11T09:08:40Z

@evilebottnawi Can you fix Node 7? It's choking on error Mixed spaces and tabs no-mixed-spaces-and-tabs at eslint.

alexander-akait · 2017-04-11T09:09:53Z

/cc @bebraw done

bebraw · 2017-04-11T09:14:52Z

lib/loader.js

 	}

 	processCss(content, map, {
+        moduleMode: moduleMode,


Why to pass moduleMode here? I don't see related code in the new tests.

@bebraw stupid error, sorry

No worries. 👍

bebraw · 2017-04-11T09:15:19Z

lib/loader.js

 		map = JSON.stringify(map);
+	} else {
+		map = null;


Will it fail without map = null?

@bebraw yep, example postcss-loader now always generate source map and send their, we should remove their

Ok, add a comment and a test so we don't lose the behavior in the future.

@bebraw caf9873#diff-bbbe73d87e5520ca3a3e6b634cfae1d9R13 I have already added a test, what else is needed?

Comment added

alexander-akait · 2017-04-11T15:18:14Z

/cc @bebraw @michael-ciniawsky @d3viant0ne I added a line prohibiting the generation source maps by postcss and it is speedup my build about ~1.5x-2x on css files (scss and etc).

michael-ciniawsky

@evilebottnawi Thx

joshwiens

Given @jhnns would be directly effected by this change and the fact he's championing the performance testing, it would be prudent to have him take a look at this as well.

michael-ciniawsky · 2017-04-13T19:36:14Z

@evilebottnawi Yeah sure 😛, I only mentioned it in case you quickly want to go through the files and convert tabs => spaces, not important for this PR feel free to ignore

joshwiens · 2017-04-14T02:20:04Z

And yes, @evilebottnawi we will get this out prior to the defaults upgrade so it is of benefit to the current major version range.

michael-ciniawsky · 2017-04-20T16:58:02Z

Guys, what's the status here, anything outstanding ? 😛

michael-ciniawsky · 2017-04-23T19:31:56Z

@evilebottnawi status :) ?

alexander-akait · 2017-04-23T19:42:11Z

@michael-ciniawsky wait answer #478 (comment)

jhnns · 2017-04-26T15:44:38Z

Sorry for being unresponsive... I'm flooded with notifications 😞

Feel free to merge this if you think that my comments have been addressed.

alexander-akait · 2017-04-26T15:53:32Z

/cc @jhnns done
/cc @michael-ciniawsky we can merge 😄

This Pull Request updates dependency [css-loader](https://github.com/webpack-contrib/css-loader) from `^0.28.11` to `^1.0.0` <details> <summary>Release Notes</summary> ### [`v1.0.0`](https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md#100httpsgithubcomwebpack-contribcss-loadercomparev02811v100-2018-07-06) [Compare Source](webpack-contrib/css-loader@v0.28.11...v1.0.0) ##### BREAKING CHANGES * remove `minimize` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`cssnano`](https://github.com/cssnano/cssnano) or use [`optimize-cssnano-plugin`](https://github.com/intervolga/optimize-cssnano-plugin) plugin * remove `module` option, use `modules` option instead * remove `camelcase` option, use `camelCase` option instead * remove `root` option, use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin * remove `alias` option, use [`resolve.alias`](https://webpack.js.org/configuration/resolve/) feature or use [`postcss-loader`](https://github.com/postcss/postcss-loader) with [`postcss-url`](https://github.com/postcss/postcss-url) plugin * update `postcss` to `6` version * minimum require `nodejs` version is `6.9` * minimum require `webpack` version is `4` #### [0.28.11](webpack-contrib/css-loader@v0.28.10...v0.28.11) (2018-03-16) ##### Bug Fixes * **lib/processCss:** don't check `mode` for `url` handling (`options.modules`) ([#698](`webpack-contrib/css-loader#698)) ([c788450](webpack-contrib/css-loader@c788450)) #### [0.28.10](webpack-contrib/css-loader@v0.28.9...v0.28.10) (2018-02-22) ##### Bug Fixes * **getLocalIdent:** add `rootContext` support (`webpack >= v4.0.0`) ([#681](`webpack-contrib/css-loader#681)) ([9f876d2](webpack-contrib/css-loader@9f876d2)) #### [0.28.9](webpack-contrib/css-loader@v0.28.8...v0.28.9) (2018-01-17) ##### Bug Fixes * ignore invalid URLs (`url()`) ([#663](`webpack-contrib/css-loader#663)) ([d1d8221](webpack-contrib/css-loader@d1d8221)) #### [0.28.8](webpack-contrib/css-loader@v0.28.7...v0.28.8) (2018-01-05) ##### Bug Fixes * **loader:** correctly check if source map is `undefined` ([#641](`webpack-contrib/css-loader#641)) ([0dccfa9](webpack-contrib/css-loader@0dccfa9)) * proper URL escaping and wrapping (`url()`) ([#627](`webpack-contrib/css-loader#627)) ([8897d44](webpack-contrib/css-loader@8897d44)) #### [0.28.7](webpack-contrib/css-loader@v0.28.6...v0.28.7) (2017-08-30) ##### Bug Fixes * pass resolver to `localsLoader` (`options.alias`) ([#601](`webpack-contrib/css-loader#601)) ([8f1b57c](webpack-contrib/css-loader@8f1b57c)) #### [0.28.6](webpack-contrib/css-loader@v0.28.5...v0.28.6) (2017-08-30) ##### Bug Fixes * add support for aliases starting with `/` (`options.alias`) ([#597](`webpack-contrib/css-loader#597)) ([63567f2](webpack-contrib/css-loader@63567f2)) #### [0.28.5](webpack-contrib/css-loader@v0.28.4...v0.28.5) (2017-08-17) ##### Bug Fixes * match mutliple dashes (`options.camelCase`) ([#556](`webpack-contrib/css-loader#556)) ([1fee601](webpack-contrib/css-loader@1fee601)) * stricter `[@import]` tolerance ([#593](`webpack-contrib/css-loader#593)) ([2e4ec09](webpack-contrib/css-loader@2e4ec09)) #### [0.28.4](webpack-contrib/css-loader@v0.28.3...v0.28.4) (2017-05-30) ##### Bug Fixes * preserve leading underscore in class names ([#543](`webpack-contrib/css-loader#543)) ([f6673c8](webpack-contrib/css-loader@f6673c8)) #### [0.28.3](webpack-contrib/css-loader@v0.28.2...v0.28.3) (2017-05-25) ##### Bug Fixes * correct plugin order for CSS Modules ([#534](`webpack-contrib/css-loader#534)) ([b90f492](webpack-contrib/css-loader@b90f492)) #### [0.28.2](webpack-contrib/css-loader@v0.28.1...v0.28.2) (2017-05-22) ##### Bug Fixes * source maps path on `windows` ([#532](`webpack-contrib/css-loader#532)) ([c3d0d91](webpack-contrib/css-loader@c3d0d91)) #### [0.28.1](webpack-contrib/css-loader@v0.28.0...v0.28.1) (2017-05-02) ##### Bug Fixes * allow to specify a full hostname as a root URL ([#521](`webpack-contrib/css-loader#521)) ([06d27a1](webpack-contrib/css-loader@06d27a1)) * case insensitivity of [@import] ([#514](`webpack-contrib/css-loader#514)) ([de4356b](webpack-contrib/css-loader@de4356b)) * don't handle empty [@import] and url() ([#513](`webpack-contrib/css-loader#513)) ([868fc94](webpack-contrib/css-loader@868fc94)) * imported variables are replaced in exports if followed by a comma ([#504](`webpack-contrib/css-loader#504)) ([956bad7](webpack-contrib/css-loader@956bad7)) * loader now correctly handles `url` with space(s) ([#495](`webpack-contrib/css-loader#495)) ([534ea55](webpack-contrib/css-loader@534ea55)) * url with a trailing space is now handled correctly ([#494](`webpack-contrib/css-loader#494)) ([e1ec4f2](webpack-contrib/css-loader@e1ec4f2)) * use `btoa` instead `Buffer` ([#501](`webpack-contrib/css-loader#501)) ([fbb0714](webpack-contrib/css-loader@fbb0714)) ##### Performance Improvements * generate source maps only when explicitly set ([#478](`webpack-contrib/css-loader#478)) ([b8f5c8f](webpack-contrib/css-loader@b8f5c8f)) --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).

michael-ciniawsky self-assigned this Apr 8, 2017

michael-ciniawsky added pr: bugfix labels Apr 8, 2017

michael-ciniawsky requested review from bebraw, michael-ciniawsky and joshwiens April 8, 2017 18:29

michael-ciniawsky approved these changes Apr 8, 2017

View reviewed changes

alexander-akait force-pushed the perf branch from 46705ba to 13a251f Compare April 11, 2017 09:05

alexander-akait force-pushed the perf branch from 13a251f to 7db4631 Compare April 11, 2017 09:09

bebraw reviewed Apr 11, 2017

View reviewed changes

alexander-akait force-pushed the perf branch 2 times, most recently from caf9873 to e8d3dc2 Compare April 11, 2017 09:26

bebraw approved these changes Apr 11, 2017

View reviewed changes

alexander-akait force-pushed the perf branch from 37799b6 to ac1eb9a Compare April 11, 2017 15:20

alexander-akait mentioned this pull request Apr 11, 2017

sass-loader performance webpack-contrib/sass-loader#296

Closed

alexander-akait force-pushed the perf branch 3 times, most recently from 4684422 to ea7456c Compare April 11, 2017 20:20

michael-ciniawsky approved these changes Apr 12, 2017

View reviewed changes

michael-ciniawsky added pr: review and removed pr: tests labels Apr 12, 2017

michael-ciniawsky added this to Perf in Dashboard Apr 12, 2017

joshwiens reviewed Apr 12, 2017

View reviewed changes

michael-ciniawsky requested a review from jhnns April 12, 2017 20:18

michael-ciniawsky changed the title ~~Fixed: generate source maps when they are actually requested.~~ fix: generate source maps only when explicitly set Apr 18, 2017

michael-ciniawsky added the semver: patch label Apr 18, 2017

michael-ciniawsky changed the title ~~fix: generate source maps only when explicitly set~~ perf: generate source maps only when explicitly set Apr 20, 2017

michael-ciniawsky removed the pr: bugfix label Apr 20, 2017

michael-ciniawsky added this to the 0.29.0 milestone Apr 20, 2017

michael-ciniawsky mentioned this pull request Apr 22, 2017

Possibility to disable css source maps webpack-contrib/extract-text-webpack-plugin#235

Closed

michael-ciniawsky added type: perf and removed pr: perf labels Apr 23, 2017

alexander-akait force-pushed the perf branch from 875c4f8 to cb1ceb3 Compare April 26, 2017 15:50

Fixed: generate source maps when they are actually requested.

7c1cb0a

alexander-akait force-pushed the perf branch from cb1ceb3 to 7c1cb0a Compare April 26, 2017 15:52

michael-ciniawsky approved these changes Apr 26, 2017

View reviewed changes

michael-ciniawsky merged commit b8f5c8f into webpack-contrib:master Apr 26, 2017

michael-ciniawsky added semver: patch and removed priority: high labels Apr 26, 2017

alexander-akait deleted the perf branch April 26, 2017 19:47

michael-ciniawsky removed this from Perf in Dashboard May 21, 2017

michael-ciniawsky modified the milestone: 0.29.0 May 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: generate source maps only when explicitly set #478

perf: generate source maps only when explicitly set #478

alexander-akait commented Apr 8, 2017

codecov bot commented Apr 8, 2017 •

edited

michael-ciniawsky left a comment

alexander-akait commented Apr 11, 2017

bebraw commented Apr 11, 2017 •

edited

alexander-akait commented Apr 11, 2017

bebraw Apr 11, 2017

alexander-akait Apr 11, 2017

bebraw Apr 11, 2017

bebraw Apr 11, 2017

alexander-akait Apr 11, 2017

bebraw Apr 11, 2017

alexander-akait Apr 11, 2017

alexander-akait Apr 11, 2017

alexander-akait commented Apr 11, 2017

michael-ciniawsky left a comment •

edited

joshwiens left a comment

michael-ciniawsky commented Apr 13, 2017

joshwiens commented Apr 14, 2017

michael-ciniawsky commented Apr 20, 2017

michael-ciniawsky commented Apr 23, 2017

alexander-akait commented Apr 23, 2017

jhnns commented Apr 26, 2017

alexander-akait commented Apr 26, 2017

Navigation Menu

perf: generate source maps only when explicitly set #478

perf: generate source maps only when explicitly set #478

Conversation

alexander-akait commented Apr 8, 2017

codecov bot commented Apr 8, 2017 • edited

Codecov Report

michael-ciniawsky left a comment

Choose a reason for hiding this comment

alexander-akait commented Apr 11, 2017

bebraw commented Apr 11, 2017 • edited

alexander-akait commented Apr 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Apr 11, 2017

michael-ciniawsky left a comment • edited

Choose a reason for hiding this comment

joshwiens left a comment

Choose a reason for hiding this comment

michael-ciniawsky commented Apr 13, 2017

joshwiens commented Apr 14, 2017

michael-ciniawsky commented Apr 20, 2017

michael-ciniawsky commented Apr 23, 2017

alexander-akait commented Apr 23, 2017

jhnns commented Apr 26, 2017

alexander-akait commented Apr 26, 2017

codecov bot commented Apr 8, 2017 •

edited

bebraw commented Apr 11, 2017 •

edited

michael-ciniawsky left a comment •

edited