refactor: export as ES2015 Module #10

SpaceK33z · 2016-10-27T09:55:58Z

Since webpack v2, it's better to use ES modules. Note that this is incompatible with webpack v1, so it should be released as a major version.

bondz · 2017-02-27T11:35:59Z

Is this going to be merged?

bebraw · 2017-03-05T15:59:17Z

It would be better to go with

// webpack 2
if (loader.version === 2) return `export default ${publicPath}`

return `module.exports = ${publicPath}`

or so. We need a standard way for checking which version of webpack is used.

Related discussion at webpack-contrib/file-loader#130 .

michael-ciniawsky · 2017-03-06T00:58:28Z

@SpaceK33z @bebraw There are few PR open from @SpaceK33z which started/proposed es2015 modules exports, the solution above it what worked for me, but would be good to be approved by @sokra 😛

michael-ciniawsky · 2017-03-06T00:59:45Z

index.js

@@ -5,5 +5,5 @@
 module.exports = function(content) {
 	this.cacheable && this.cacheable();
 	this.value = content;


Unrelated to the PR but what is this line actually doing 🤔

this.value = content

Good question. It's some webpack 0.9 fix from 9bbe8b2 . I wonder if that line is even needed now.

Unrelated, but from my debugging session, this assignment does not actually occur, as afterwards, this.value remains undefined. Or, actually, not defined at all.

Another note. this.cacheable is set by default now so it would be possible to drop that.

bebraw · 2017-03-06T06:12:59Z

Can you sign the CLA? It has to be signed separately for the contrib org. That + fallback for webpack 1 + possible removal of this.value bit after that is understood would make it possible to merge this.

bebraw · 2017-03-06T06:59:13Z

We had a quick chat about this. Basically return "export default " + JSON.stringify(content); is enough. You should, however, set a peer dependency against webpack 2 and publish the change as a major version.

Branching would bring unnecessary trouble so a clean break is neater for the users. If you go webpack 2, better go all the way.

mightyiam · 2017-03-07T15:02:06Z

index.js

@@ -5,5 +5,5 @@
 module.exports = function(content) {
 	this.cacheable && this.cacheable();
 	this.value = content;
-	return "module.exports = " + JSON.stringify(content);
+	return "export default " + JSON.stringify(content);


Is this why it isn't working for me with import foo from './foo'?

I had to do

import * as foo from './foo'

which is very wrong... If this will correct behavior, please accept...

Confirmed working with webpack v2.2.1.

mightyiam

Works for me with babel v2.2.1.

mightyiam · 2017-03-07T15:06:09Z

index.js

@@ -5,5 +5,5 @@
 module.exports = function(content) {
 	this.cacheable && this.cacheable();
 	this.value = content;
-	return "module.exports = " + JSON.stringify(content);
+	return "export default " + JSON.stringify(content);


I had to do

import * as foo from './foo'

which is very wrong... If this will correct behavior, please accept...

Confirmed working with webpack v2.2.1.

michael-ciniawsky · 2017-03-07T17:06:47Z

@mightyiam Thats possible 😛 did you disable babel modules transformations?

SpaceK33z · 2017-03-07T17:37:16Z

@bebraw I signed the CLA. Is this safe to merge now?

bebraw · 2017-03-07T17:42:43Z

@SpaceK33z Let's wait for a review from Joshua. It's a simple change, but better play it safe.

joshwiens · 2017-03-07T18:35:53Z

Squash & merge with the following commit message ( matters for defaults changelog stuff )

feat: Export as ES Module

BREAKING CHANGE: ES Modules are not compatible with Webpack v1.x

Worth noting, this should be released in conjunction with the webpack-defaults update as it enforces node > 4.3 and is technically breaking as well.

Added to milestone 1.0.0-beta.0 and should be published as the beta tag to be safe ( silly for 6 lines of code, I know ).

mightyiam · 2017-03-07T18:40:31Z

@michael-ciniawsky I wasn't using babel. I wasn't combining it with any other loader, if that's what you mean.

michael-ciniawsky · 2017-03-07T18:43:38Z

@mightyiam Yep, that's what I meant, forget about it then 😛

joshwiens · 2017-03-07T18:45:55Z

@bebraw - Pretty sure @mightyiam is talking about doing a standard named import as opposed to doing import * as

@mightyiam - That will require a slightly different set of changes that will happen as a part of webpack-defaults update and be included in the same MAJOR version release as the ES module change here.

After defaults, the transpiled code would look something like this ...

"use strict";

Object.defineProperty(exports, "__esModule", {
  value: true
});

exports.default = function (content) {
  this.cacheable && this.cacheable();
  this.value = content;
  return `export default ${JSON.stringify(content)}`;
};

mightyiam · 2017-03-14T17:30:28Z

Is something blocking this?

bebraw · 2017-03-14T17:34:24Z

@mightyiam Not really. Just busy times for a lot of people.

joshwiens · 2017-03-15T03:52:50Z

@mightyiam - This has to be released as a semver MAJOR, we have another breaking change that needs to be merge ready before this is merged & released.

mightyiam · 2017-09-26T05:02:26Z

Any update?

joshwiens · 2017-10-01T01:25:31Z

@mightyiam - This will still require #25 before it finds it's way out to npm. I'll get that PR updates shortly for a final once over.

mightyiam · 2017-10-01T09:44:15Z

Thank you!

feat: Export as ES Module BREAKING CHANGE: ES Modules are not compatible with Webpack v1.x

Export an ES Module instead of CommonJS module

20854ca

Since webpack v2, it's better to use ES modules. Note that this is incompatible with webpack v1, so it should be released as a major version.

michael-ciniawsky reviewed Mar 6, 2017

View reviewed changes

mightyiam reviewed Mar 7, 2017

View reviewed changes

mightyiam approved these changes Mar 7, 2017

View reviewed changes

bebraw requested a review from joshwiens March 7, 2017 17:42

joshwiens approved these changes Mar 7, 2017

View reviewed changes

joshwiens added this to the 1.0.0-beta.0 milestone Mar 7, 2017

joshwiens self-assigned this Mar 8, 2017

joshwiens removed their assignment Aug 17, 2017

michael-ciniawsky changed the title ~~Export an ES Module instead of CommonJS module~~ feat: export as ES2015 Module Sep 27, 2017

michael-ciniawsky added priority: 4 (important) semver: Major type: Refactor pr_action: Blocked labels Sep 27, 2017

michael-ciniawsky added the status: Approved label Sep 27, 2017

michael-ciniawsky changed the title ~~feat: export as ES2015 Module~~ refactor: export as ES2015 Module Sep 27, 2017

joshwiens merged commit 215fba7 into webpack-contrib:master Oct 1, 2017

michael-ciniawsky mentioned this pull request Oct 1, 2017

fix: escape newline/paragraph separators #36

Merged

michael-ciniawsky removed priority: 4 (important) pr_action: Blocked labels Oct 1, 2017

joshwiens pushed a commit that referenced this pull request Nov 20, 2017

refactor: export as ES2015 Module (#10)

a293c06

feat: Export as ES Module BREAKING CHANGE: ES Modules are not compatible with Webpack v1.x

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: export as ES2015 Module #10

refactor: export as ES2015 Module #10

SpaceK33z commented Oct 27, 2016

bondz commented Feb 27, 2017

bebraw commented Mar 5, 2017

michael-ciniawsky commented Mar 6, 2017

michael-ciniawsky Mar 6, 2017 •

edited

Loading

bebraw Mar 6, 2017

mightyiam Mar 7, 2017

bebraw Mar 7, 2017

bebraw commented Mar 6, 2017

bebraw commented Mar 6, 2017

mightyiam Mar 7, 2017

mightyiam Mar 7, 2017

mightyiam left a comment

mightyiam Mar 7, 2017

michael-ciniawsky commented Mar 7, 2017

SpaceK33z commented Mar 7, 2017

bebraw commented Mar 7, 2017

joshwiens commented Mar 7, 2017 •

edited

Loading

mightyiam commented Mar 7, 2017

michael-ciniawsky commented Mar 7, 2017 •

edited

Loading

joshwiens commented Mar 7, 2017 •

edited

Loading

mightyiam commented Mar 14, 2017

bebraw commented Mar 14, 2017

joshwiens commented Mar 15, 2017

mightyiam commented Sep 26, 2017

joshwiens commented Oct 1, 2017

mightyiam commented Oct 1, 2017

refactor: export as ES2015 Module #10

refactor: export as ES2015 Module #10

Conversation

SpaceK33z commented Oct 27, 2016

bondz commented Feb 27, 2017

bebraw commented Mar 5, 2017

michael-ciniawsky commented Mar 6, 2017

michael-ciniawsky Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

bebraw Mar 6, 2017

Choose a reason for hiding this comment

mightyiam Mar 7, 2017

Choose a reason for hiding this comment

bebraw Mar 7, 2017

Choose a reason for hiding this comment

bebraw commented Mar 6, 2017

bebraw commented Mar 6, 2017

mightyiam Mar 7, 2017

Choose a reason for hiding this comment

mightyiam Mar 7, 2017

Choose a reason for hiding this comment

mightyiam left a comment

Choose a reason for hiding this comment

mightyiam Mar 7, 2017

Choose a reason for hiding this comment

michael-ciniawsky commented Mar 7, 2017

SpaceK33z commented Mar 7, 2017

bebraw commented Mar 7, 2017

joshwiens commented Mar 7, 2017 • edited Loading

mightyiam commented Mar 7, 2017

michael-ciniawsky commented Mar 7, 2017 • edited Loading

joshwiens commented Mar 7, 2017 • edited Loading

mightyiam commented Mar 14, 2017

bebraw commented Mar 14, 2017

joshwiens commented Mar 15, 2017

mightyiam commented Sep 26, 2017

joshwiens commented Oct 1, 2017

mightyiam commented Oct 1, 2017

michael-ciniawsky Mar 6, 2017 •

edited

Loading

joshwiens commented Mar 7, 2017 •

edited

Loading

michael-ciniawsky commented Mar 7, 2017 •

edited

Loading

joshwiens commented Mar 7, 2017 •

edited

Loading