Merge pull request #694 from sveltejs/oncreate-before-bindings

Call `oncreate` functions before updating bindings
sveltejs · Jul 7, 2017 · 459ca95 · 459ca95
2 parents ef33466 + d89d70e
commit 459ca95
Show file tree

Hide file tree

Showing 23 changed files with 177 additions and 42 deletions.
diff --git a/src/generators/dom/index.ts b/src/generators/dom/index.ts
@@ -122,10 +122,10 @@ export default function dom(
 		@dispatchObservers( this, this._observers.pre, newState, oldState );
 		${block.hasUpdateMethod && `this._fragment.update( newState, this._state );`}
 		@dispatchObservers( this, this._observers.post, newState, oldState );
-		${generator.hasComplexBindings &&
-			`while ( this._bindings.length ) this._bindings.pop()();`}
 		${(generator.hasComponents || generator.hasIntroTransitions) &&
 			`this._flush();`}
+		${generator.hasComplexBindings &&
+			`while ( this._bindings.length ) this._bindings.pop()();`}
 	`;
 
 	if (hasJs) {
@@ -200,7 +200,7 @@ export default function dom(
 				options.css !== false &&
 				`if ( !document.getElementById( '${generator.cssId}-style' ) ) @add_css();`}
 			${(generator.hasComponents || generator.hasIntroTransitions) &&
-				`this._renderHooks = [];`}
+				`this._oncreate = [];`}
 			${generator.hasComplexBindings && `this._bindings = [];`}
 
 			this._fragment = @create_main_fragment( this._state, this );
@@ -219,15 +219,15 @@ export default function dom(
 				this._fragment.${block.hasIntroMethod ? 'intro' : 'mount'}( options.target, null );
 			}
 			
-			${generator.hasComplexBindings &&
-				`while ( this._bindings.length ) this._bindings.pop()();`}
 			${(generator.hasComponents || generator.hasIntroTransitions) &&
 				`this._flush();`}
+			${generator.hasComplexBindings &&
+				`while ( this._bindings.length ) this._bindings.pop()();`}
 
 			${templateProperties.oncreate &&
 				deindent`
 				if ( options._root ) {
-					options._root._renderHooks.push( @template.oncreate.bind( this ) );
+					options._root._oncreate.push( @template.oncreate.bind( this ) );
 				} else {
 					@template.oncreate.call( this );
 				}

diff --git a/src/generators/dom/visitors/Element/addTransitions.ts b/src/generators/dom/visitors/Element/addTransitions.ts
@@ -23,7 +23,7 @@ export default function addTransitions(
 		const fn = `@template.transitions.${intro.name}`;
 
 		block.builders.intro.addBlock(deindent`
-			#component._renderHooks.push( function () {
+			#component._oncreate.push( function () {
 				if ( !${name} ) ${name} = @wrapTransition( ${state.name}, ${fn}, ${snippet}, true, null );
 				${name}.run( true, function () {
 					#component.fire( 'intro.end', { node: ${state.name} });
@@ -58,7 +58,7 @@ export default function addTransitions(
 			}
 
 			block.builders.intro.addBlock(deindent`
-				#component._renderHooks.push( function () {
+				#component._oncreate.push( function () {
 					${introName} = @wrapTransition( ${state.name}, ${fn}, ${snippet}, true, null );
 					${introName}.run( true, function () {
 						#component.fire( 'intro.end', { node: ${state.name} });

diff --git a/src/shared/index.js b/src/shared/index.js
@@ -110,10 +110,10 @@ export function set(newState) {
 }
 
 export function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/js/samples/collapses-text-around-comments/expected-bundle.js b/test/js/samples/collapses-text-around-comments/expected-bundle.js
@@ -119,10 +119,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/js/samples/computed-collapsed-if/expected-bundle.js b/test/js/samples/computed-collapsed-if/expected-bundle.js
@@ -95,10 +95,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/js/samples/each-block-changed-check/expected-bundle.js b/test/js/samples/each-block-changed-check/expected-bundle.js
@@ -128,10 +128,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/js/samples/event-handlers-custom/expected-bundle.js b/test/js/samples/event-handlers-custom/expected-bundle.js
@@ -113,10 +113,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/js/samples/if-block-no-update/expected-bundle.js b/test/js/samples/if-block-no-update/expected-bundle.js
@@ -119,10 +119,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/js/samples/if-block-simple/expected-bundle.js b/test/js/samples/if-block-simple/expected-bundle.js
@@ -119,10 +119,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/js/samples/non-imported-component/expected-bundle.js b/test/js/samples/non-imported-component/expected-bundle.js
@@ -107,10 +107,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 
@@ -183,14 +183,15 @@ function SvelteComponent ( options ) {
 	this._yield = options._yield;
 
 	this._torndown = false;
-	this._renderHooks = [];
+	this._oncreate = [];
 
 	this._fragment = create_main_fragment( this._state, this );
 
 	if ( options.target ) {
 		this._fragment.create();
 		this._fragment.mount( options.target, null );
 	}
+
 	this._flush();
 }
 

diff --git a/test/js/samples/non-imported-component/expected.js b/test/js/samples/non-imported-component/expected.js
@@ -62,14 +62,15 @@ function SvelteComponent ( options ) {
 	this._yield = options._yield;
 
 	this._torndown = false;
-	this._renderHooks = [];
+	this._oncreate = [];
 
 	this._fragment = create_main_fragment( this._state, this );
 
 	if ( options.target ) {
 		this._fragment.create();
 		this._fragment.mount( options.target, null );
 	}
+
 	this._flush();
 }
 

diff --git a/test/js/samples/onrender-onteardown-rewritten/expected-bundle.js b/test/js/samples/onrender-onteardown-rewritten/expected-bundle.js
@@ -95,10 +95,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 
@@ -156,7 +156,7 @@ function SvelteComponent ( options ) {
 	}
 
 	if ( options._root ) {
-		options._root._renderHooks.push( template.oncreate.bind( this ) );
+		options._root._oncreate.push( template.oncreate.bind( this ) );
 	} else {
 		template.oncreate.call( this );
 	}

diff --git a/test/js/samples/onrender-onteardown-rewritten/expected.js b/test/js/samples/onrender-onteardown-rewritten/expected.js
@@ -45,7 +45,7 @@ function SvelteComponent ( options ) {
 	}
 
 	if ( options._root ) {
-		options._root._renderHooks.push( template.oncreate.bind( this ) );
+		options._root._oncreate.push( template.oncreate.bind( this ) );
 	} else {
 		template.oncreate.call( this );
 	}

diff --git a/test/js/samples/use-elements-as-anchors/expected-bundle.js b/test/js/samples/use-elements-as-anchors/expected-bundle.js
@@ -119,10 +119,10 @@ function set(newState) {
 }
 
 function _flush() {
-	if (!this._renderHooks) return;
+	if (!this._oncreate) return;
 
-	while (this._renderHooks.length) {
-		this._renderHooks.pop()();
+	while (this._oncreate.length) {
+		this._oncreate.pop()();
 	}
 }
 

diff --git a/test/runtime/samples/bindings-before-oncreate/One.html b/test/runtime/samples/bindings-before-oncreate/One.html
@@ -0,0 +1,15 @@
+<Two bind:foo/>
+
+<script>
+	import Two from './Two.html';
+
+	export default {
+		components: {
+			Two
+		},
+
+		oncreate() {
+			this.snapshot = this.get('foo');
+		}
+	};
+</script>
diff --git a/test/runtime/samples/bindings-before-oncreate/Two.html b/test/runtime/samples/bindings-before-oncreate/Two.html
@@ -0,0 +1,15 @@
+<script>
+	export default {
+		data() {
+			return {
+				bar: 1
+			};
+		},
+
+		computed: {
+			foo(bar) {
+				return bar * 2;
+			}
+		}
+	};
+</script>
diff --git a/test/runtime/samples/bindings-before-oncreate/_config.js b/test/runtime/samples/bindings-before-oncreate/_config.js
@@ -0,0 +1,5 @@
+export default {
+	test(assert, component) {
+		assert.equal(component.refs.one.snapshot, 2);
+	}
+};
diff --git a/test/runtime/samples/bindings-before-oncreate/main.html b/test/runtime/samples/bindings-before-oncreate/main.html
@@ -0,0 +1,9 @@
+<One ref:one/>
+
+<script>
+	import One from './One.html';
+
+	export default {
+		components: { One }
+	};
+</script>
diff --git a/test/runtime/samples/flush-before-bindings/Nested.html b/test/runtime/samples/flush-before-bindings/Nested.html
@@ -0,0 +1,30 @@
+{{#each things as thing}}
+	<Visibility bind:isVisible="visibilityMap[thing]">
+		<p>{{thing}} ({{visibilityMap[thing]}})</p>
+	</Visibility> 
+{{/each}}
+
+<script>
+	import Visibility from './Visibility.html';
+	import counter from './counter.js';
+
+	export default {
+		data() {
+			return {
+				things: ['first thing', 'second thing'],
+				visibilityMap: {}
+			};
+		},
+
+		computed: {
+			visibleThings: (things, visibilityMap) => {
+				counter.count += 1;
+				return things.filter(text => visibilityMap[text]);
+			}
+		},
+
+		components: {
+			Visibility
+		}
+	};
+</script>
diff --git a/test/runtime/samples/flush-before-bindings/Visibility.html b/test/runtime/samples/flush-before-bindings/Visibility.html
@@ -0,0 +1,11 @@
+<div>
+	{{yield}}
+</div>
+
+<script>
+	export default {
+		oncreate() {
+			this.set({ isVisible: true });
+		}
+	};
+</script>
diff --git a/test/runtime/samples/flush-before-bindings/_config.js b/test/runtime/samples/flush-before-bindings/_config.js
@@ -0,0 +1,21 @@
+import counter from './counter.js';
+
+export default {
+	'skip-ssr': true,
+
+	html: `
+		<div><p>first thing (true)</p></div>
+		<div><p>second thing (true)</p></div>
+	`,
+
+	test(assert, component) {
+		const visibleThings = component.get('visibleThings');
+		assert.deepEqual(visibleThings, ['first thing', 'second thing']);
+
+		const snapshots = component.snapshots;
+		assert.deepEqual(snapshots, [visibleThings]);
+
+		// TODO minimise the number of recomputations during oncreate
+		// assert.equal(counter.count, 1);
+	}
+};
diff --git a/test/runtime/samples/flush-before-bindings/counter.js b/test/runtime/samples/flush-before-bindings/counter.js
@@ -0,0 +1,3 @@
+export default {
+	count: 0
+};