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.

Sign up for GitHub

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

Jump to bottom

feat: library target UMD supports names per target #4883

Merged
merged 5 commits into from
Jun 21, 2017
Merged

feat: library target UMD supports names per target #4883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0ab7b1a
feat: library target UMD supports names per target
AndersDJohnson May 17, 2017
2ec4b5c
fix: update schema to support library as object
AndersDJohnson May 29, 2017
d87fb0e
chore: remove temp files
AndersDJohnson May 29, 2017
369efb3
test: add for namedDefine true and false
AndersDJohnson May 29, 2017
913b18b
fix: update hashes for all library names
AndersDJohnson May 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 24 additions & 10 deletions lib/UmdMainTemplatePlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,17 @@ function accessorAccess(base, accessor) {

class UmdMainTemplatePlugin {
constructor(name, options) {
this.name = name;
if(typeof name === "object" && !Array.isArray(name)) {
this.name = name.root || name.amd || name.commonjs;
this.names = name;
} else {
Copy link
Member

Choose a reason for hiding this comment

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

does the schema allow an object as output.libraryName?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sokra Good point. Updated schema and added test.

this.name = name;
this.names = {
commonjs: name,
root: name,
amd: name
};
}
this.optionalAmdExternalAsGlobal = options.optionalAmdExternalAsGlobal;
this.namedDefine = options.namedDefine;
this.auxiliaryComment = options.auxiliaryComment;
Expand Down Expand Up @@ -124,16 +134,16 @@ class UmdMainTemplatePlugin {
) +
" else if(typeof define === 'function' && define.amd)\n" +
(requiredExternals.length > 0 ?
(this.name && this.namedDefine === true ?
" define(" + libraryName(this.name) + ", " + externalsDepsArray(requiredExternals) + ", " + amdFactory + ");\n" :
(this.names.amd && this.namedDefine === true ?
" define(" + libraryName(this.names.amd) + ", " + externalsDepsArray(requiredExternals) + ", " + amdFactory + ");\n" :
" define(" + externalsDepsArray(requiredExternals) + ", " + amdFactory + ");\n"
) :
(this.name && this.namedDefine === true ?
" define(" + libraryName(this.name) + ", [], " + amdFactory + ");\n" :
(this.names.amd && this.namedDefine === true ?
" define(" + libraryName(this.names.amd) + ", [], " + amdFactory + ");\n" :
Copy link
Member

Choose a reason for hiding this comment

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

this line is not tested, if you want to you can add a test, but it's not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sokra Thanks, I've added tests for namedDefine being true and false. I'm not sure why it was a string value before, as I'm not sure that's a valid value per the docs - I'd only copied that from other test cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sokra Not sure how to get that line covered. Looks like it wasn't before either, and this PR now improves coverage overall. https://codecov.io/gh/webpack/webpack/pull/4883/changes

" define([], " + amdFactory + ");\n"
)
) +
(this.name ?
(this.names.root || this.names.commonjs ?
(this.auxiliaryComment &&
typeof this.auxiliaryComment === "string" ?
" //" + this.auxiliaryComment + "\n" :
Expand All @@ -142,7 +152,7 @@ class UmdMainTemplatePlugin {
""
) +
" else if(typeof exports === 'object')\n" +
" exports[" + libraryName(this.name) + "] = factory(" + externalsRequireArray("commonjs") + ");\n" +
" exports[" + libraryName(this.names.commonjs || this.names.root) + "] = factory(" + externalsRequireArray("commonjs") + ");\n" +
(this.auxiliaryComment &&
typeof this.auxiliaryComment === "string" ?
" //" + this.auxiliaryComment + "\n" :
Expand All @@ -151,7 +161,7 @@ class UmdMainTemplatePlugin {
""
) +
" else\n" +
" " + replaceKeys(accessorAccess("root", this.name)) + " = factory(" + externalsRootArray(externals) + ");\n" :
" " + replaceKeys(accessorAccess("root", this.names.root || this.names.commonjs)) + " = factory(" + externalsRootArray(externals) + ");\n" :
" else {\n" +
(externals.length > 0 ?
" var a = typeof exports === 'object' ? factory(" + externalsRequireArray("commonjs") + ") : factory(" + externalsRootArray(externals) + ");\n" :
Expand All @@ -163,12 +173,16 @@ class UmdMainTemplatePlugin {
"})(this, function(" + externalsArguments(externals) + ") {\nreturn ", "webpack/universalModuleDefinition"), source, ";\n})");
}.bind(this));
mainTemplate.plugin("global-hash-paths", function(paths) {
if(this.name) paths = paths.concat(this.name);
if(this.names.root) paths = paths.concat(this.names.root);
Copy link
Member

Choose a reason for hiding this comment

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

Could you add all names here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sokra Is it okay if names are repeated in paths, or do we need to dedupe first?

Copy link
Member

Choose a reason for hiding this comment

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

That's ok. They should just contribute all to the hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sokra Again here, the paths have been added for all names.

if(this.names.amd) paths = paths.concat(this.names.amd);
if(this.names.commonjs) paths = paths.concat(this.names.commonjs);
return paths;
}.bind(this));
mainTemplate.plugin("hash", function(hash) {
hash.update("umd");
hash.update(`${this.name}`);
hash.update(`${this.names.root}`);
Copy link
Member

Choose a reason for hiding this comment

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

here also all names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sokra Again, is it okay if names are repeated here, or do we need to dedupe first?

Copy link
Member

Choose a reason for hiding this comment

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

yep, it's ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sokra The hash has been updated with all names.

hash.update(`${this.names.amd}`);
hash.update(`${this.names.commonjs}`);
}.bind(this));
}
}
Expand Down
8 changes: 8 additions & 0 deletions schemas/webpackOptionsSchema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,14 @@
"type": "string"
},
"type": "array"
},
{
"type": "object",
"properties": {
"root": { "type": "string" },
"amd": { "type": "string" },
"commonjs": { "type": "string" }
}
}
],
"description": "If set, export the bundle as library. `output.library` is the name."
Expand Down
83 changes: 83 additions & 0 deletions test/LibraryTemplatePlugin.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,89 @@ describe("LibraryTemplatePlugin", function() {
});
});
});

describe("name is an object of names per target", function() {
[{
type: "umd",
namedDefine: true,
assertion: function(compilationContext) {
compilationContext.name.should.be.exactly("barRoot");
compilationContext.names.should.deepEqual({
root: "barRoot",
amd: "bar-amd"
});
compilationContext.optionalAmdExternalAsGlobal.should.be.false();
compilationContext.namedDefine.should.be.exactly(true);
compilationContext.auxiliaryComment.should.be.exactly("baz");
}
},
{
type: "umd",
namedDefine: false,
assertion: function(compilationContext) {
compilationContext.name.should.be.exactly("barRoot");
compilationContext.names.should.deepEqual({
root: "barRoot",
amd: "bar-amd"
});
compilationContext.optionalAmdExternalAsGlobal.should.be.false();
compilationContext.namedDefine.should.be.exactly(false);
compilationContext.auxiliaryComment.should.be.exactly("baz");
}
},
{
type: "umd2",
namedDefine: true,
assertion: function(compilationContext) {
compilationContext.name.should.be.exactly("barRoot");
compilationContext.names.should.deepEqual({
root: "barRoot",
amd: "bar-amd"
});
compilationContext.optionalAmdExternalAsGlobal.should.be.true();
compilationContext.namedDefine.should.be.exactly(true);
compilationContext.auxiliaryComment.should.be.exactly("baz");
}
},
{
type: "umd2",
namedDefine: false,
assertion: function(compilationContext) {
compilationContext.name.should.be.exactly("barRoot");
compilationContext.names.should.deepEqual({
root: "barRoot",
amd: "bar-amd"
});
compilationContext.optionalAmdExternalAsGlobal.should.be.true();
compilationContext.namedDefine.should.be.exactly(false);
compilationContext.auxiliaryComment.should.be.exactly("baz");
}
}
].forEach(function(targetTypeAndAssertion) {
var type = targetTypeAndAssertion.type;
var namedDefine = targetTypeAndAssertion.namedDefine;

describe("when target is " + type, function() {
beforeEach(function() {
env.eventBindings = applyPluginWithOptions(LibraryTemplatePlugin, {
root: "barRoot",
amd: "bar-amd"
}, type, namedDefine, "baz");
env.eventBinding = env.eventBindings[0];
env.eventBinding.handler(env.compilation);
});

it("compilation callback is called", function() {
env.compilation.callCount.should.be.exactly(1);
});

it("compilation callback context is set up", function() {
var compilationContext = env.compilation.firstCall.thisValue;
targetTypeAndAssertion.assertion(compilationContext);
});
});
});
});
});
});
});
3 changes: 3 additions & 0 deletions test/configCases/library/umd/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
it("should run", function() {

});
10 changes: 10 additions & 0 deletions test/configCases/library/umd/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = {
output: {
libraryTarget: "umd",
library: {
root: "testLibrary",
amd: "test-library",
commonjs: "test-library"
}
}
};