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 UMD wrapper issues and refine wrappers #2600

Merged
merged 4 commits into from Dec 19, 2018
Merged

Conversation

lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Dec 19, 2018

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

List any relevant issue numbers:

Description

With the recent changes to the UMD wrapper, several bugs were introduced. This addresses this and includes a thorough test suite of 240 tests (of which 84 were broken, some event before the latest refactorings!) that actually runs the UMD wrapper with different combinations of options and different scenarios:

options:

  • simple global variable or namespaced.variable.with@special/characters
  • compact or non-compact mode
  • iife noConflict method present or not

environments:

  • node
  • simulated amd
  • simulated iife where this = global variable
  • simulated iife strict mode where this = undefined but self = global variable
  • simulated iife with pre-existing bundle variable (to test the noConflict method)

tests:

  • no imports/exports
  • named exports
  • default exports
  • import + named exports
  • import + default export

All tests are green now and the wrapper could be further refined. Moreover, this will now have precedence over self in iife mode and the check has been moved into the wrapper code, e.g.

(function (global, factory) {
	typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('foo'), require('bar')) :
	typeof define === 'function' && define.amd ? define(['foo', 'bar'], factory) :
	(global = global || self, factory(global.foo));
}(this, function (foo) { 'use strict';

	console.log( foo.a );

}));

A similar treatment is done for the IIFE wrappers.

@thysultan
Copy link

Was there any reason there for the assignment in (global = global || self, factory(global.foo)); instead of factory((global || self).foo)?

@lukastaegert
Copy link
Member Author

lukastaegert commented Dec 19, 2018

The reason for doing it in a little more complicated way is that in most situations, global will be used more than once and it was getting complicated to check how often global is accessed and implement two different versions for each possible access. Getting this right would have meant basically counting all accesses to global first before actually writing anything. Not impossible but quite complicated.

@lukastaegert
Copy link
Member Author

For reference, this is what the wrapper will look like in a complicated scenario with a noConflict method, nested variable name my.nested.bundle, two imports and a named export:

(function (global, factory) {
	typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('external1'), require('external2')) :
	typeof define === 'function' && define.amd ? define(['exports', 'external1', 'external2'], factory) :
	(global = global || self, (function () {
		var current = global.my && global.my.nested && global.my.nested.bundle;
		var exports = (global.my = global.my || {}, global.my.nested = global.my.nested || {}, global.my.nested.bundle = {});
		factory(exports, global.external1, global.external2);
		exports.noConflict = function () { global.my.nested.bundle = current; return exports; };
	}()));
}(this, function (exports, external1, external2) { 'use strict';

	const x = external1.value1 || external2.value2;

	exports.x = x;

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

}));

@lukastaegert lukastaegert merged commit 8f71071 into master Dec 19, 2018
@lukastaegert lukastaegert deleted the refactor-umd branch December 19, 2018 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants