fix(cjs): export `raw` value #183

alexander-akait · 2017-07-27T17:41:24Z

We use "main": "dist/cjs.js" in package.json, but default(https://github.com/webpack-contrib/file-loader/blob/master/src/cjs.js#L1) doesn't contain raw value so file-loader doesn't work correctly.

codecov · 2017-07-27T17:49:11Z

Codecov Report

Merging #183 into master will increase coverage by 3.03%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #183      +/-   ##
========================================
+ Coverage   96.96%   100%   +3.03%     
========================================
  Files           2      2              
  Lines          33     35       +2     
  Branches       16     16              
========================================
+ Hits           32     35       +3     
+ Misses          1      0       -1

Impacted Files	Coverage Δ
src/cjs.js	`100% <100%> (+100%)`	⬆️

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 0c52ae2...f9b839e. Read the comment docs.

michael-ciniawsky · 2017-07-27T18:40:32Z

src/cjs.js

@@ -1 +1,4 @@
-module.exports = require('./index').default;
+const fileLoader = require('./index');


fileLoader => loader 😛

@michael-ciniawsky 👍 done

Jessidhia · 2017-07-28T01:30:01Z

src/cjs.js

+const fileLoader = require('./index');
+
+module.exports = fileLoader.default;
+module.exports.raw = fileLoader.raw;


This is mutating fileLoader.default... 😕

Maybe it could be better to write the workaround in index.js itself, with a comment that it's a workaround for the commonjs export.

They'll both do the same thing, and I guess technically put the cjs change outside of the cjs wrapper, but I'd prefer that to being mutated from outside.

@Kovensky Perhaps, but on the other hand, I would not want to add workarounds to the loader code, because it can be misleading. In any case, I'm open to ideas, I'll be glad to see another solution

@Kovensky What are potential issues with mutating loader.default ? Handling the Module System related stuff in the wrapper would be 👍 if possible 🙃

alexander-akait · 2017-09-09T16:55:05Z

@michael-ciniawsky just ensure tests are good, also npm have bug with node 4.3 😞 npm/npm#18395

Use hotfixed file-loader for now, until webpack-contrib/file-loader#183 is released.

brianhelba · 2017-09-29T22:50:46Z

@evilebottnawi See #204 for a fix to tests.

michael-ciniawsky · 2017-09-29T23:14:24Z

@brianhelba This needs to be updated in webpack-defaults instead as it is the boilerplate for all repos

@d3viant0ne @evilebottnawi If it is the only known failure blocking, can we merge the fix regardless of the npm CI incompat for now and fix it in a follow up PR? Setting raw is fairly important for the loader

brianhelba · 2017-09-29T23:30:50Z

@michael-ciniawsky I agree, I'd like to merge this PR ASAP, even without the fix in #204. The CI tests are already failing on master, so this won't make anything worse (and this still passes tests on everything except Node.js 4.3). However, this does fix a very serious regression, which makes file-loader totally unusable for binary files (e.g. images) right now.

@d3viant0ne @evilebottnawi Is that ok?

joshwiens · 2017-09-30T02:40:36Z

@brianhelba - file-loader@1.1.2

brianhelba · 2017-09-30T02:42:16Z

Excellent! Thanks for the quick attention and release.

alexander-akait added priority: 2 (urgent) semver: Minor severity: 2 (regression) type: Bug labels Jul 27, 2017

alexander-akait requested review from bebraw, michael-ciniawsky and joshwiens July 27, 2017 17:41

michael-ciniawsky approved these changes Jul 27, 2017

View reviewed changes

michael-ciniawsky changed the title ~~fix: exports raw value in cjs~~ fix(cjs): export raw value Jul 27, 2017

michael-ciniawsky added this to the 1.0.0 milestone Jul 27, 2017

Jessidhia reviewed Jul 28, 2017

View reviewed changes

alexander-akait force-pushed the fix-exports-raw-value branch from fb94f98 to 8d1e15f Compare July 28, 2017 11:42

michael-ciniawsky added pr_action: Review status: Pending labels Jul 28, 2017

alexander-akait mentioned this pull request Jul 31, 2017

broken jpg #185

Closed

cainlevy mentioned this pull request Aug 1, 2017

inconsistent export syntax when used with file-loader@1.0 webpack-contrib/url-loader#85

Closed

alexander-akait mentioned this pull request Sep 9, 2017

fix: "raw = true" was not used by webpack #193

Closed

alexander-akait removed the request for review from bebraw September 9, 2017 15:02

joshwiens approved these changes Sep 9, 2017

View reviewed changes

This was referenced Sep 10, 2017

feat: update minimum required node.js version to 4.8 for travis CI webpack-contrib/webpack-defaults#82

Merged

Add note about JSX usage #35

Closed

beheh added a commit to HearthSim/twitch-hdt-frontend that referenced this pull request Sep 23, 2017

Add card tooltips based on react-hs-components

d76351d

Use hotfixed file-loader for now, until webpack-contrib/file-loader#183 is released.

This was referenced Sep 29, 2017

Some images not display #188

Closed

build(travis): update minimum node v4.3.0...4.8.0 #204

Closed

michael-ciniawsky mentioned this pull request Sep 29, 2017

Corrupted images emitted #205

Closed

michael-ciniawsky mentioned this pull request Sep 30, 2017

file-loader corrupts output images webpack/webpack#5745

Closed

michael-ciniawsky modified the milestones: 2.0.0, 1.1.1 Sep 30, 2017

michael-ciniawsky added semver: Patch and removed semver: Minor labels Sep 30, 2017

evilebottnawi added 2 commits September 29, 2017 21:27

fix: exports raw value in cjs

52b2514

test: add cjs tests

f9b839e

joshwiens force-pushed the fix-exports-raw-value branch from 7d7ec84 to f9b839e Compare September 30, 2017 02:27

joshwiens merged commit daeff0e into master Sep 30, 2017

joshwiens deleted the fix-exports-raw-value branch September 30, 2017 02:39

michael-ciniawsky added status: Approved and removed priority: 2 (urgent) pr_action: Review severity: 2 (regression) status: Pending labels Sep 30, 2017

michael-ciniawsky removed this from the 1.1.3 milestone Sep 30, 2017

romansp mentioned this pull request Oct 18, 2017

skpm/file-loader corrupts output files skpm/skpm#80

Closed

dependencies bot mentioned this pull request Dec 5, 2017

Update 23 dependencies from npm JetBrains/ring-ui#31

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cjs): export `raw` value #183

fix(cjs): export `raw` value #183

alexander-akait commented Jul 27, 2017 •

edited

codecov bot commented Jul 27, 2017 •

edited

michael-ciniawsky Jul 27, 2017

alexander-akait Jul 28, 2017

Jessidhia Jul 28, 2017

alexander-akait Jul 28, 2017

michael-ciniawsky Jul 28, 2017 •

edited

alexander-akait commented Sep 9, 2017

brianhelba commented Sep 29, 2017

michael-ciniawsky commented Sep 29, 2017

brianhelba commented Sep 29, 2017

joshwiens commented Sep 30, 2017

brianhelba commented Sep 30, 2017

		@@ -1 +1,4 @@
		module.exports = require('./index').default;
		const fileLoader = require('./index');

fix(cjs): export raw value #183

fix(cjs): export raw value #183

Conversation

alexander-akait commented Jul 27, 2017 • edited

codecov bot commented Jul 27, 2017 • edited

Codecov Report

michael-ciniawsky Jul 27, 2017

Choose a reason for hiding this comment

alexander-akait Jul 28, 2017

Choose a reason for hiding this comment

Jessidhia Jul 28, 2017

Choose a reason for hiding this comment

alexander-akait Jul 28, 2017

Choose a reason for hiding this comment

michael-ciniawsky Jul 28, 2017 • edited

Choose a reason for hiding this comment

alexander-akait commented Sep 9, 2017

brianhelba commented Sep 29, 2017

michael-ciniawsky commented Sep 29, 2017

brianhelba commented Sep 29, 2017

joshwiens commented Sep 30, 2017

brianhelba commented Sep 30, 2017

fix(cjs): export `raw` value #183

fix(cjs): export `raw` value #183

alexander-akait commented Jul 27, 2017 •

edited

codecov bot commented Jul 27, 2017 •

edited

michael-ciniawsky Jul 28, 2017 •

edited