From 728d2fa9fb870be4fa70bc5e5401e78c87ee668a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 10 Jan 2018 21:48:03 -0500 Subject: [PATCH] deconflict referenced globals - fixes #1079 --- package.json | 3 + src/generators/Generator.ts | 9 +- src/generators/dom/index.ts | 2 +- src/utils/annotateWithScopes.ts | 10 +- .../deconflict-globals/expected-bundle.js | 221 ++++++++++++++++++ .../js/samples/deconflict-globals/expected.js | 52 +++++ test/js/samples/deconflict-globals/input.html | 11 + yarn.lock | 10 + 8 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 test/js/samples/deconflict-globals/expected-bundle.js create mode 100644 test/js/samples/deconflict-globals/expected.js create mode 100644 test/js/samples/deconflict-globals/input.html diff --git a/package.json b/package.json index a2d4890cd7..07a1e9e103 100644 --- a/package.json +++ b/package.json @@ -87,5 +87,8 @@ ], "sourceMap": true, "instrument": true + }, + "dependencies": { + "is-reference": "^1.1.0" } } diff --git a/src/generators/Generator.ts b/src/generators/Generator.ts index f0c36f8062..ab74b40c0d 100644 --- a/src/generators/Generator.ts +++ b/src/generators/Generator.ts @@ -410,11 +410,16 @@ export default class Generator { const indentationLevel = getIndentationLevel(source, js.content.body[0].start); const indentExclusionRanges = getIndentExclusionRanges(js.content); - const scope = annotateWithScopes(js.content); + const { scope, globals } = annotateWithScopes(js.content); + scope.declarations.forEach(name => { this.userVars.add(name); }); + globals.forEach(name => { + this.userVars.add(name); + }); + const body = js.content.body.slice(); // slice, because we're going to be mutating the original // imports need to be hoisted out of the IIFE @@ -666,7 +671,7 @@ export default class Generator { isEventHandler: boolean ) => { this.addSourcemapLocations(node); // TODO this involves an additional walk — can we roll it in somewhere else? - let scope = annotateWithScopes(node); + let { scope } = annotateWithScopes(node); const dependencies: Set = new Set(); diff --git a/src/generators/dom/index.ts b/src/generators/dom/index.ts index 3a21e356f5..c124f05bb5 100644 --- a/src/generators/dom/index.ts +++ b/src/generators/dom/index.ts @@ -407,7 +407,7 @@ export default function dom( const code = new MagicString(str); const expression = parseExpressionAt(str, 0); - let scope = annotateWithScopes(expression); + let { scope } = annotateWithScopes(expression); walk(expression, { enter(node: Node, parent: Node) { diff --git a/src/utils/annotateWithScopes.ts b/src/utils/annotateWithScopes.ts index 9b5c9d5e88..41e734b6e9 100644 --- a/src/utils/annotateWithScopes.ts +++ b/src/utils/annotateWithScopes.ts @@ -1,11 +1,13 @@ import { walk } from 'estree-walker'; +import isReference from 'is-reference'; import { Node } from '../interfaces'; export default function annotateWithScopes(expression: Node) { + const globals = new Set(); let scope = new Scope(null, false); walk(expression, { - enter(node: Node) { + enter(node: Node, parent: Node) { if (/Function/.test(node.type)) { if (node.type === 'FunctionDeclaration') { scope.declarations.add(node.id.name); @@ -25,6 +27,10 @@ export default function annotateWithScopes(expression: Node) { node._scope = scope = new Scope(scope, true); } else if (/(Function|Class|Variable)Declaration/.test(node.type)) { scope.addDeclaration(node); + } else if (isReference(node, parent)) { + if (!scope.has(node.name)) { + globals.add(node.name); + } } }, @@ -35,7 +41,7 @@ export default function annotateWithScopes(expression: Node) { }, }); - return scope; + return { scope, globals }; } export class Scope { diff --git a/test/js/samples/deconflict-globals/expected-bundle.js b/test/js/samples/deconflict-globals/expected-bundle.js new file mode 100644 index 0000000000..ea55a11851 --- /dev/null +++ b/test/js/samples/deconflict-globals/expected-bundle.js @@ -0,0 +1,221 @@ +function noop() {} + +function assign(target) { + var k, + source, + i = 1, + len = arguments.length; + for (; i < len; i++) { + source = arguments[i]; + for (k in source) target[k] = source[k]; + } + + return target; +} + +function blankObject() { + return Object.create(null); +} + +function destroy(detach) { + this.destroy = noop; + this.fire('destroy'); + this.set = this.get = noop; + + if (detach !== false) this._fragment.u(); + this._fragment.d(); + this._fragment = this._state = null; +} + +function differs(a, b) { + return a !== b || ((a && typeof a === 'object') || typeof a === 'function'); +} + +function dispatchObservers(component, group, changed, newState, oldState) { + for (var key in group) { + if (!changed[key]) continue; + + var newValue = newState[key]; + var oldValue = oldState[key]; + + var callbacks = group[key]; + if (!callbacks) continue; + + for (var i = 0; i < callbacks.length; i += 1) { + var callback = callbacks[i]; + if (callback.__calling) continue; + + callback.__calling = true; + callback.call(component, newValue, oldValue); + callback.__calling = false; + } + } +} + +function fire(eventName, data) { + var handlers = + eventName in this._handlers && this._handlers[eventName].slice(); + if (!handlers) return; + + for (var i = 0; i < handlers.length; i += 1) { + handlers[i].call(this, data); + } +} + +function get(key) { + return key ? this._state[key] : this._state; +} + +function init(component, options) { + component._observers = { pre: blankObject(), post: blankObject() }; + component._handlers = blankObject(); + component._bind = options._bind; + + component.options = options; + component.root = options.root || component; + component.store = component.root.store || options.store; +} + +function observe(key, callback, options) { + var group = options && options.defer + ? this._observers.post + : this._observers.pre; + + (group[key] || (group[key] = [])).push(callback); + + if (!options || options.init !== false) { + callback.__calling = true; + callback.call(this, this._state[key]); + callback.__calling = false; + } + + return { + cancel: function() { + var index = group[key].indexOf(callback); + if (~index) group[key].splice(index, 1); + } + }; +} + +function on(eventName, handler) { + if (eventName === 'teardown') return this.on('destroy', handler); + + var handlers = this._handlers[eventName] || (this._handlers[eventName] = []); + handlers.push(handler); + + return { + cancel: function() { + var index = handlers.indexOf(handler); + if (~index) handlers.splice(index, 1); + } + }; +} + +function set(newState) { + this._set(assign({}, newState)); + if (this.root._lock) return; + this.root._lock = true; + callAll(this.root._beforecreate); + callAll(this.root._oncreate); + callAll(this.root._aftercreate); + this.root._lock = false; +} + +function _set(newState) { + var oldState = this._state, + changed = {}, + dirty = false; + + for (var key in newState) { + if (differs(newState[key], oldState[key])) changed[key] = dirty = true; + } + if (!dirty) return; + + this._state = assign({}, oldState, newState); + this._recompute(changed, this._state); + if (this._bind) this._bind(changed, this._state); + + if (this._fragment) { + dispatchObservers(this, this._observers.pre, changed, this._state, oldState); + this._fragment.p(changed, this._state); + dispatchObservers(this, this._observers.post, changed, this._state, oldState); + } +} + +function callAll(fns) { + while (fns && fns.length) fns.pop()(); +} + +function _mount(target, anchor) { + this._fragment.m(target, anchor); +} + +function _unmount() { + if (this._fragment) this._fragment.u(); +} + +var proto = { + destroy: destroy, + get: get, + fire: fire, + observe: observe, + on: on, + set: set, + teardown: destroy, + _recompute: noop, + _set: _set, + _mount: _mount, + _unmount: _unmount +}; + +/* generated by Svelte vX.Y.Z */ +function data_1() { + return { + foo: 'bar' +}; +} + +function oncreate() { + alert(JSON.stringify(data())); +} + +function create_main_fragment(state, component) { + + return { + c: noop, + + m: noop, + + p: noop, + + u: noop, + + d: noop + }; +} + +function SvelteComponent(options) { + init(this, options); + this._state = assign(data_1(), options.data); + + var _oncreate = oncreate.bind(this); + + if (!options.root) { + this._oncreate = [_oncreate]; + } else { + this.root._oncreate.push(_oncreate); + } + + this._fragment = create_main_fragment(this._state, this); + + if (options.target) { + this._fragment.c(); + this._fragment.m(options.target, options.anchor || null); + + callAll(this._oncreate); + } +} + +assign(SvelteComponent.prototype, proto); + +export default SvelteComponent; diff --git a/test/js/samples/deconflict-globals/expected.js b/test/js/samples/deconflict-globals/expected.js new file mode 100644 index 0000000000..d08d3dc9a1 --- /dev/null +++ b/test/js/samples/deconflict-globals/expected.js @@ -0,0 +1,52 @@ +/* generated by Svelte vX.Y.Z */ +import { assign, callAll, init, noop, proto } from "svelte/shared.js"; + +function data_1() { + return { + foo: 'bar' +}; +} + +function oncreate() { + alert(JSON.stringify(data())); +}; + +function create_main_fragment(state, component) { + + return { + c: noop, + + m: noop, + + p: noop, + + u: noop, + + d: noop + }; +} + +function SvelteComponent(options) { + init(this, options); + this._state = assign(data_1(), options.data); + + var _oncreate = oncreate.bind(this); + + if (!options.root) { + this._oncreate = [_oncreate]; + } else { + this.root._oncreate.push(_oncreate); + } + + this._fragment = create_main_fragment(this._state, this); + + if (options.target) { + this._fragment.c(); + this._fragment.m(options.target, options.anchor || null); + + callAll(this._oncreate); + } +} + +assign(SvelteComponent.prototype, proto); +export default SvelteComponent; \ No newline at end of file diff --git a/test/js/samples/deconflict-globals/input.html b/test/js/samples/deconflict-globals/input.html new file mode 100644 index 0000000000..6c3dc88e87 --- /dev/null +++ b/test/js/samples/deconflict-globals/input.html @@ -0,0 +1,11 @@ + \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index bf6229baef..1ea0b19b77 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,6 +2,10 @@ # yarn lockfile v1 +"@types/estree@0.0.38": + version "0.0.38" + resolved "https://registry.yarnpkg.com/@types/estree/-/estree-0.0.38.tgz#c1be40aa933723c608820a99a373a16d215a1ca2" + "@types/mocha@^2.2.41": version "2.2.44" resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-2.2.44.tgz#1d4a798e53f35212fd5ad4d04050620171cd5b5e" @@ -1635,6 +1639,12 @@ is-property@^1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/is-property/-/is-property-1.0.2.tgz#57fe1c4e48474edd65b09911f26b1cd4095dda84" +is-reference@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/is-reference/-/is-reference-1.1.0.tgz#50e6ef3f64c361e2c53c0416cdc9420037f2685b" + dependencies: + "@types/estree" "0.0.38" + is-resolvable@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/is-resolvable/-/is-resolvable-1.0.0.tgz#8df57c61ea2e3c501408d100fb013cf8d6e0cc62"