From 8ee498b9d9d3e052a1291111a0a4a893550ebd50 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 13 Nov 2017 20:46:45 -0500 Subject: [PATCH 01/16] failing test for #908 --- .../_config.js | 61 +++++++++++++++++++ .../main.html | 6 ++ 2 files changed, 67 insertions(+) create mode 100644 test/runtime/samples/globals-not-overwritten-by-bindings/_config.js create mode 100644 test/runtime/samples/globals-not-overwritten-by-bindings/main.html diff --git a/test/runtime/samples/globals-not-overwritten-by-bindings/_config.js b/test/runtime/samples/globals-not-overwritten-by-bindings/_config.js new file mode 100644 index 000000000000..0774ada96217 --- /dev/null +++ b/test/runtime/samples/globals-not-overwritten-by-bindings/_config.js @@ -0,0 +1,61 @@ +export default { + html: ` +
+ + +
+ +
+ + +
+ +
+ + +
+ `, + + data: { + todos: { + first: { + description: 'Buy some milk', + done: true, + }, + second: { + description: 'Do the laundry', + done: true, + }, + third: { + description: "Find life's true purpose", + done: true, + }, + }, + }, + + test(assert, component, target, window) { + const input = document.querySelectorAll('input[type="checkbox"]')[2]; + const change = new window.Event('change'); + + input.checked = true; + input.dispatchEvent(change); + + assert.ok(component.get('todos').third.done); + assert.htmlEqual(target.innerHTML, ` +
+ + +
+ +
+ + +
+ +
+ + +
+ `); + }, +}; diff --git a/test/runtime/samples/globals-not-overwritten-by-bindings/main.html b/test/runtime/samples/globals-not-overwritten-by-bindings/main.html new file mode 100644 index 000000000000..ca6c6ccb3545 --- /dev/null +++ b/test/runtime/samples/globals-not-overwritten-by-bindings/main.html @@ -0,0 +1,6 @@ +{{#each Object.keys(todos) as key}} +
+ + +
+{{/each}} \ No newline at end of file From 8c467bcb88a4510adb0db54daa27509aff9c8f1b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 21 Nov 2017 23:12:02 -0500 Subject: [PATCH 02/16] add globals to initial state object --- src/generators/Generator.ts | 16 ++------------ src/generators/dom/index.ts | 18 +++++++++++++++- src/generators/server-side-rendering/index.ts | 21 ++++++++++++++++--- .../_config.js | 2 +- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/generators/Generator.ts b/src/generators/Generator.ts index 8f2dad24e4cf..7e1c52eac32e 100644 --- a/src/generators/Generator.ts +++ b/src/generators/Generator.ts @@ -6,7 +6,6 @@ import CodeBuilder from '../utils/CodeBuilder'; import getCodeFrame from '../utils/getCodeFrame'; import isReference from '../utils/isReference'; import flattenReference from '../utils/flattenReference'; -import globalWhitelist from '../utils/globalWhitelist'; import reservedNames from '../utils/reservedNames'; import namespaces from '../utils/namespaces'; import { removeNode, removeObjectKey } from '../utils/removeNode'; @@ -267,16 +266,7 @@ export default class Generator { } } - if (globalWhitelist.has(name)) { - code.prependRight(node.start, `('${name}' in state ? state.`); - code.appendLeft( - node.object ? node.object.end : node.end, - ` : ${name})` - ); - } else { - code.prependRight(node.start, `state.`); - } - + code.prependRight(node.start, `state.`); usedContexts.add('state'); } @@ -349,9 +339,7 @@ export default class Generator { }); dependencies.forEach(name => { - if (!globalWhitelist.has(name)) { - this.expectedProperties.add(name); - } + this.expectedProperties.add(name); }); return (expression._dependencies = dependencies); diff --git a/src/generators/dom/index.ts b/src/generators/dom/index.ts index 7c7c4a45dc8d..1cbf6132cfa6 100644 --- a/src/generators/dom/index.ts +++ b/src/generators/dom/index.ts @@ -6,6 +6,7 @@ import { walk } from 'estree-walker'; import deindent from '../../utils/deindent'; import { stringify, escape } from '../../utils/stringify'; import CodeBuilder from '../../utils/CodeBuilder'; +import globalWhitelist from '../../utils/globalWhitelist'; import reservedNames from '../../utils/reservedNames'; import visit from './visit'; import shared from './shared'; @@ -184,13 +185,28 @@ export default function dom( const debugName = `<${generator.customElement ? generator.tag : name}>`; + // generate initial state object + const globals = Array.from(generator.expectedProperties).filter(prop => globalWhitelist.has(prop)); + const initialState = []; + if (globals.length > 0) { + initialState.push(`{ ${globals.map(prop => `${prop} : ${prop}`).join(', ')} }`); + } + + if (templateProperties.data) { + initialState.push(`%data()`); + } else if (globals.length === 0) { + initialState.push('{}'); + } + + initialState.push(`options.data`); + const constructorBody = deindent` ${options.dev && `this._debugName = '${debugName}';`} ${options.dev && !generator.customElement && `if (!options || (!options.target && !options._root)) throw new Error("'target' is a required option");`} @init(this, options); ${generator.usesRefs && `this.refs = {};`} - this._state = @assign(${templateProperties.data ? '%data()' : '{}'}, options.data); + this._state = @assign(${initialState.join(', ')}); ${generator.metaBindings} ${computations.length && `this._recompute({ ${Array.from(computationDeps).map(dep => `${dep}: 1`).join(', ')} }, this._state);`} ${options.dev && diff --git a/src/generators/server-side-rendering/index.ts b/src/generators/server-side-rendering/index.ts index b4590df95d2e..2accc2bc7e8f 100644 --- a/src/generators/server-side-rendering/index.ts +++ b/src/generators/server-side-rendering/index.ts @@ -6,6 +6,7 @@ import preprocess from './preprocess'; import visit from './visit'; import { removeNode, removeObjectKey } from '../../utils/removeNode'; import getName from '../../utils/getName'; +import globalWhitelist from '../../utils/globalWhitelist'; import { Parsed, Node, CompileOptions } from '../../interfaces'; import { AppendTarget } from './interfaces'; import { stringify } from '../../utils/stringify'; @@ -71,6 +72,22 @@ export default function ssr( { css: null, cssMap: null } : generator.stylesheet.render(options.filename, true); + // generate initial state object + // TODO this doesn't work, because expectedProperties isn't populated + const globals = Array.from(generator.expectedProperties).filter(prop => globalWhitelist.has(prop)); + const initialState = []; + if (globals.length > 0) { + initialState.push(`{ ${globals.map(prop => `${prop} : ${prop}`).join(', ')} }`); + } + + if (templateProperties.data) { + initialState.push(`%data()`); + } else if (globals.length === 0) { + initialState.push('{}'); + } + + initialState.push('state'); + const result = deindent` ${generator.javascript} @@ -83,9 +100,7 @@ export default function ssr( }; ${name}.render = function(state, options) { - ${templateProperties.data - ? `state = Object.assign(%data(), state || {});` - : `state = state || {};`} + state = Object.assign(${initialState.join(', ')}); ${computations.map( ({ key, deps }) => diff --git a/test/runtime/samples/globals-not-overwritten-by-bindings/_config.js b/test/runtime/samples/globals-not-overwritten-by-bindings/_config.js index 0774ada96217..a10750e1a035 100644 --- a/test/runtime/samples/globals-not-overwritten-by-bindings/_config.js +++ b/test/runtime/samples/globals-not-overwritten-by-bindings/_config.js @@ -28,7 +28,7 @@ export default { }, third: { description: "Find life's true purpose", - done: true, + done: false, }, }, }, From 30345ea0dbe94bc355e1e545465c01900a1ca343 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Wed, 22 Nov 2017 11:31:40 -0500 Subject: [PATCH 03/16] start refactoring contextualise/findDependencies --- package.json | 2 +- src/generators/Generator.ts | 156 ++++++++++++++++-- src/generators/dom/Block.ts | 8 - src/generators/dom/preprocess.ts | 24 +-- src/generators/dom/visitors/Component.ts | 9 +- src/generators/dom/visitors/EachBlock.ts | 2 +- .../dom/visitors/Element/Attribute.ts | 7 +- .../dom/visitors/Element/StyleAttribute.ts | 3 +- .../dom/visitors/Element/addBindings.ts | 3 +- src/generators/dom/visitors/shared/Tag.ts | 5 +- src/utils/annotateWithScopes.ts | 2 +- .../ssr-no-oncreate-etc/expected-bundle.js | 2 +- .../samples/ssr-no-oncreate-etc/expected.js | 2 +- yarn.lock | 4 +- 14 files changed, 172 insertions(+), 57 deletions(-) diff --git a/package.json b/package.json index a57d6cdedf1f..361cee6b2383 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "eslint": "^4.3.0", "eslint-plugin-html": "^3.0.0", "eslint-plugin-import": "^2.2.0", - "estree-walker": "^0.5.0", + "estree-walker": "^0.5.1", "glob": "^7.1.1", "jsdom": "^11.1.0", "locate-character": "^2.0.0", diff --git a/src/generators/Generator.ts b/src/generators/Generator.ts index 7e1c52eac32e..4016c71edb8f 100644 --- a/src/generators/Generator.ts +++ b/src/generators/Generator.ts @@ -10,7 +10,7 @@ import reservedNames from '../utils/reservedNames'; import namespaces from '../utils/namespaces'; import { removeNode, removeObjectKey } from '../utils/removeNode'; import wrapModule from './shared/utils/wrapModule'; -import annotateWithScopes from '../utils/annotateWithScopes'; +import annotateWithScopes, { Scope } from '../utils/annotateWithScopes'; import getName from '../utils/getName'; import clone from '../utils/clone'; import DomBlock from './dom/Block'; @@ -155,7 +155,12 @@ export default class Generator { this.aliases = new Map(); this.usedNames = new Set(); - this.parseJs(dom); + this.computations = []; + this.templateProperties = {}; + + this.walkJs(dom); + this.walkTemplate(); + this.name = this.alias(name); if (options.customElement === true) { @@ -208,7 +213,7 @@ export default class Generator { const { code, helpers } = this; const { contexts, indexes } = block; - let scope = annotateWithScopes(expression); // TODO this already happens in findDependencies + let scope: Scope; let lexicalDepth = 0; const self = this; @@ -230,7 +235,7 @@ export default class Generator { }); } else if (isReference(node, parent)) { const { name } = flattenReference(node); - if (scope.has(name)) return; + if (scope && scope.has(name)) return; if (name === 'event' && isEventHandler) { // noop @@ -307,10 +312,10 @@ export default class Generator { ) { if (expression._dependencies) return expression._dependencies; - let scope = annotateWithScopes(expression); + let scope: Scope; const dependencies: string[] = []; - const generator = this; // can't use arrow functions, because of this.skip() + const { helpers } = this; // can't use arrow functions, because of this.skip() walk(expression, { enter(node: Node, parent: Node) { @@ -321,7 +326,7 @@ export default class Generator { if (isReference(node, parent)) { const { name } = flattenReference(node); - if (scope.has(name) || generator.helpers.has(name)) return; + if (scope && scope.has(name) || helpers.has(name)) return; if (contextDependencies.has(name)) { dependencies.push(...contextDependencies.get(name)); @@ -442,17 +447,19 @@ export default class Generator { }; } - parseJs(dom: boolean) { - const { code, source } = this; + walkJs(dom: boolean) { + const { + code, + source, + computations, + templateProperties, + imports + } = this; + const { js } = this.parsed; - const imports = this.imports; - const computations: Computation[] = []; - const templateProperties: Record = {}; const componentDefinition = new CodeBuilder(); - let namespace = null; - if (js) { this.addSourcemapLocations(js.content); @@ -625,7 +632,7 @@ export default class Generator { if (templateProperties.namespace) { const ns = templateProperties.namespace.value.value; - namespace = namespaces[ns] || ns; + this.namespace = namespaces[ns] || ns; } if (templateProperties.onrender) templateProperties.oncreate = templateProperties.onrender; // remove after v2 @@ -681,9 +688,122 @@ export default class Generator { this.javascript = a === b ? null : `[✂${a}-${b}✂]`; } } + } + + walkTemplate() { + const { + expectedProperties, + helpers + } = this; + const { html } = this.parsed; + + function findDependencies(node: Node, contextDependencies: Map, indexes: Set) { + const dependencies: Set = new Set(); + + let scope = annotateWithScopes(html); + + walk(node, { + enter(node: Node, parent: Node) { + if (node._scope) { + scope = node._scope; + return; + } + + if (isReference(node, parent)) { + const { name } = flattenReference(node); + if (scope && scope.has(name) || helpers.has(name)) return; + + if (contextDependencies.has(name)) { + contextDependencies.get(name).forEach(dependency => { + dependencies.add(dependency); + }); + } else if (!indexes.has(name)) { + dependencies.add(name); + } + + this.skip(); + } + }, + + leave(node: Node, parent: Node) { + if (node._scope) scope = scope.parent; + } + }); + + dependencies.forEach(dependency => { + expectedProperties.add(dependency); + }); + + return Array.from(dependencies); + } + + const contextStack = []; + const indexStack = []; + const dependenciesStack = []; + + let contextDependencies = new Map(); + const contextDependenciesStack: Map[] = [contextDependencies]; + + let indexes = new Set(); + const indexesStack: Set[] = [indexes]; - this.computations = computations; - this.namespace = namespace; - this.templateProperties = templateProperties; + walk(html, { + enter(node: Node, parent: Node) { + if (node.type === 'EachBlock') { + node.dependencies = findDependencies(node.expression, contextDependencies, indexes); + + contextDependencies = new Map(contextDependencies); + contextDependencies.set(node.context, node.dependencies); + + // if (node.destructuredContexts) { + // for (let i = 0; i < node.destructuredContexts.length; i += 1) { + // contexts.set(node.destructuredContexts[i], `${context}[${i}]`); + // contextDependencies.set(node.destructuredContexts[i], dependencies); + // } + // } + + contextDependenciesStack.push(contextDependencies); + + if (node.index) { + indexes = new Set(indexes); + indexes.add(node.index); + indexesStack.push(indexes); + } + } + + if (node.type === 'IfBlock') { + node.dependencies = findDependencies(node.expression, contextDependencies, indexes); + } + + if (node.type === 'MustacheTag' || node.type === 'RawMustacheTag' || node.type === 'AttributeShorthand') { + node.dependencies = findDependencies(node.expression, contextDependencies, indexes); + this.skip(); + } + + if (node.type === 'Binding') { + node.dependencies = findDependencies(node.value, contextDependencies, indexes); + this.skip(); + } + + if (node.type === 'EventHandler' && node.expression) { + node.expression.arguments.forEach((arg: Node) => { + arg.dependencies = findDependencies(arg, contextDependencies, indexes); + }); + this.skip(); + } + }, + + leave(node: Node, parent: Node) { + if (node.type === 'EachBlock') { + contextDependenciesStack.pop(); + contextDependencies = contextDependenciesStack[contextDependenciesStack.length - 1]; + + if (node.index) { + indexesStack.pop(); + indexes = indexesStack[indexesStack.length - 1]; + } + } + } + }); } } diff --git a/src/generators/dom/Block.ts b/src/generators/dom/Block.ts index 53ae5194650e..1c08ce7bfc9b 100644 --- a/src/generators/dom/Block.ts +++ b/src/generators/dom/Block.ts @@ -176,14 +176,6 @@ export default class Block { ); } - findDependencies(expression: Node) { - return this.generator.findDependencies( - this.contextDependencies, - this.indexes, - expression - ); - } - mount(name: string, parentNode: string) { if (parentNode) { this.builders.mount.addLine(`@appendNode(${name}, ${parentNode});`); diff --git a/src/generators/dom/preprocess.ts b/src/generators/dom/preprocess.ts index 33a305146f40..57fda53e954a 100644 --- a/src/generators/dom/preprocess.ts +++ b/src/generators/dom/preprocess.ts @@ -74,9 +74,7 @@ const preprocessors = { ) => { cannotUseInnerHTML(node); node.var = block.getUniqueName('text'); - - const dependencies = block.findDependencies(node.expression); - block.addDependencies(dependencies); + block.addDependencies(node.dependencies); }, RawMustacheTag: ( @@ -90,9 +88,7 @@ const preprocessors = { ) => { cannotUseInnerHTML(node); node.var = block.getUniqueName('raw'); - - const dependencies = block.findDependencies(node.expression); - block.addDependencies(dependencies); + block.addDependencies(node.dependencies); }, Text: ( @@ -133,8 +129,7 @@ const preprocessors = { function attachBlocks(node: Node) { node.var = block.getUniqueName(`if_block`); - const dependencies = block.findDependencies(node.expression); - block.addDependencies(dependencies); + block.addDependencies(node.dependencies); node._block = block.child({ comment: createDebuggingComment(node, generator), @@ -209,7 +204,7 @@ const preprocessors = { cannotUseInnerHTML(node); node.var = block.getUniqueName(`each`); - const dependencies = block.findDependencies(node.expression); + const { dependencies } = node; block.addDependencies(dependencies); const indexNames = new Map(block.indexNames); @@ -319,7 +314,7 @@ const preprocessors = { if (chunk.type !== 'Text') { if (node.parent) cannotUseInnerHTML(node.parent); - const dependencies = block.findDependencies(chunk.expression); + const dependencies = chunk.dependencies; block.addDependencies(dependencies); // special case —