Implement "prepareStateForRender" — Fixes #178 #181

yuchi · 2016-12-16T00:29:17Z

Let Soy components define "prepareStateForRender" to manipulate data:

While in JSXComponents there’s a place to compute derived values from the state just before rendering (that’s the "render" method), on Soy components there’s no way to do the same.

This change makes the Soy renderer to call a "prepareStateForRender" method on the component, passing the data that would be used for the template and trusting the implementation to do one of the following:

mutate the original template data and return it;
mutate the original template data and return undefined;
create a new object and return it (eventually mixing in the original data).

Fixes #178

mairatma

Thanks a lot @yuchi, this will be very useful :D

I've only added one inline comment, and besides that can you just rename your commits so that they follow the format we use? We basically write commit names as if they've started with the word If, so in your case they'd be something like: Applies source formatting, Lets ... and Add tests for "prepareStateForRender".

mairatma · 2016-12-16T00:51:14Z

packages/metal-soy/test/Soy.js

@@ -111,6 +112,27 @@ describe('Soy', function() {
 					done();
 				});
 			});
+
+			it('should pass state values to "getTemplateData" and use them in the template', function(done) {


This still uses the old getTemplateData name :)

yuchi · 2016-12-16T00:55:33Z

Sorry I'm used to if this commit is applied it will….

While in JSXComponents there’s a place to compute derived values from the state just before rendering (that’s the "render" method), on Soy components there’s no way to do the same. This change makes the Soy renderer to call a "prepareStateForRender" method on the component, passing the data that would be used for the template and trusting the implementation to do one of the following: - mutate the original template data and return it; - mutate the original template data and return undefined; - create a new object and return it (eventually mixing in the original data). Fixes metal#178

yuchi · 2016-12-16T09:38:03Z

packages/metal-soy/src/Soy.js

@@ -60,7 +62,12 @@ class Soy extends IncrementalDomRenderer.constructor {
 				data[params[i]] = component[params[i]].bind(component);
 			}
 		}
-		return data;
+
+		if (isFunction(component.prepareStateForRender)) {


@mairatma Didn’t want to make data mutable, so I’m branching the return here. Tell me if this is ok with you.

Sure, that's fine.

yuchi · 2016-12-16T09:38:57Z

packages/metal-soy/test/Soy.js

@@ -11,6 +11,7 @@ import { Events as EventsComponent } from './assets/Events.soy.js';
 // on it.
 // TODO: We should have a better dependency management for soy files so that
 // the order in which they're required doesn't matter.
+import { ComputedData as ComputedDataComponent } from './assets/ComputedData.soy.js';


Could you make sure this is the right line to place the import? I simply sorted them alphabetically.

Yes, that's correct, I've just realized that this TODO isn't necessary anymore, all imports can already be in alphabetical order today :)

yuchi · 2016-12-16T09:39:21Z

packages/metal-soy/test/Soy.js

@@ -111,6 +112,27 @@ describe('Soy', function() {
 					done();
 				});
 			});
+
+			it('should pass state values to "getTemplateData" and use them in the template', function(done) {


mairatma · 2016-12-16T17:07:02Z

Just started reviewing :)

Sent from GH.

mairatma

Thanks a lot, merging now!

mairatma · 2016-12-16T17:06:17Z

packages/metal-soy/test/Soy.js

@@ -11,6 +11,7 @@ import { Events as EventsComponent } from './assets/Events.soy.js';
 // on it.
 // TODO: We should have a better dependency management for soy files so that
 // the order in which they're required doesn't matter.
+import { ComputedData as ComputedDataComponent } from './assets/ComputedData.soy.js';


Yes, that's correct, I've just realized that this TODO isn't necessary anymore, all imports can already be in alphabetical order today :)

mairatma · 2016-12-16T17:06:37Z

packages/metal-soy/src/Soy.js

@@ -60,7 +62,12 @@ class Soy extends IncrementalDomRenderer.constructor {
 				data[params[i]] = component[params[i]].bind(component);
 			}
 		}
-		return data;
+
+		if (isFunction(component.prepareStateForRender)) {


Sure, that's fine.

fernandosouza · 2017-01-31T17:23:34Z

Hey, guys. What do you think to add a documentation for this method in MetalJS website? I can do it.

cc. @yuchi, @jbalsas.

yuchi · 2017-01-31T17:56:35Z

If you could it would be great!

fernandosouza · 2017-01-31T18:04:16Z

Of course, I can. For me, it seems a Lifecycle method. Isn't it?

yuchi · 2017-01-31T18:14:00Z

It

is a lifecycle method specific for Soy components
enables computed data without wiring events and sync* stuff
doesn't automatically renders on changes on computed properties so you still need to use the source properties in the template to have changes reflected in the UI (PR to solve that coming)

fernandosouza · 2017-01-31T18:22:10Z

Thank you, @yuchi. I'm going to write it. 😃

mairatma reviewed Dec 16, 2016

View reviewed changes

yuchi added 3 commits December 16, 2016 10:31

Applies source formatting

ffe0d0e

Add tests for "prepareStateForRender"

970e6a6

yuchi force-pushed the feature-prepare-state-for-render branch from 70c4cd8 to 970e6a6 Compare December 16, 2016 09:37

yuchi commented Dec 16, 2016

View reviewed changes

mairatma approved these changes Dec 16, 2016

View reviewed changes

mairatma merged commit 970e6a6 into metal:master Dec 16, 2016

brunobasto mentioned this pull request Jan 31, 2017

Stops manipulate the items attribute itself to render HTML in SOY template #7 deprecate/metal-select#10

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement "prepareStateForRender" — Fixes #178 #181

Implement "prepareStateForRender" — Fixes #178 #181

yuchi commented Dec 16, 2016

mairatma left a comment •

edited

Loading

mairatma Dec 16, 2016

yuchi Dec 16, 2016

yuchi Dec 16, 2016

yuchi commented Dec 16, 2016

yuchi Dec 16, 2016

mairatma Dec 16, 2016

yuchi Dec 16, 2016

mairatma Dec 16, 2016

yuchi Dec 16, 2016

mairatma commented Dec 16, 2016

mairatma left a comment

mairatma Dec 16, 2016

mairatma Dec 16, 2016

fernandosouza commented Jan 31, 2017

yuchi commented Jan 31, 2017

fernandosouza commented Jan 31, 2017

yuchi commented Jan 31, 2017

fernandosouza commented Jan 31, 2017

Implement "prepareStateForRender" — Fixes #178 #181

Implement "prepareStateForRender" — Fixes #178 #181

Conversation

yuchi commented Dec 16, 2016

mairatma left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuchi commented Dec 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mairatma commented Dec 16, 2016

mairatma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fernandosouza commented Jan 31, 2017

yuchi commented Jan 31, 2017

fernandosouza commented Jan 31, 2017

yuchi commented Jan 31, 2017

fernandosouza commented Jan 31, 2017

mairatma left a comment •

edited

Loading