Skip to content

tables should run with their own scope#119

Merged
tbouron merged 1 commit into
apache:masterfrom
ahgittin:fix-table
Mar 7, 2019
Merged

tables should run with their own scope#119
tbouron merged 1 commit into
apache:masterfrom
ahgittin:fix-table

Conversation

@ahgittin
Copy link
Copy Markdown
Contributor

without this if you put two tables in the same parent controller view they collide on their data and have their own data

without this if you put _two_ tables in the same parent controller view they collide on their data and have their own data
@tbouron
Copy link
Copy Markdown
Member

tbouron commented Feb 26, 2019

I'm really not sure about the implication of this @ahgittin. Some tables have custom templates for columns that use the current scope to get the data from. Creating a child scope for each table will break this pattern

@ahgittin
Copy link
Copy Markdown
Contributor Author

As discussed, I think you are right, it is a little more subtle (1) if attrs refer to parent scope vars, and (2) if templates refer to parent scope vars. But we do want enough separation/isolation to be able to put multiple tables in the same view without them clobbering each other.

Will continue investigating.

@ahgittin ahgittin changed the title tables should run with their own scope [WIP] tables should run with their own scope Feb 28, 2019
@ahgittin
Copy link
Copy Markdown
Contributor Author

ahgittin commented Mar 1, 2019

the effect of scope: true is to create a new scope that prototypically inherits from the parent scope. i think this is what we want in order to have the table's scope.ctrl be unique from another instance of table in the parent.

it can still see all fields from the parent that were defined when the table was created. in worst case, a caller/parent may have to initialize a field in the scope prior to creating the table (eg in the parent's controller) whereas previously they did not (since the scope was shared) and scope fields could be lazilly initialized.

i've tested with complex tables and with a one line fix to initialize a scope field to be a null list, it works fine with {scope: true}.

@ahgittin ahgittin changed the title [WIP] tables should run with their own scope tables should run with their own scope Mar 1, 2019
@tbouron
Copy link
Copy Markdown
Member

tbouron commented Mar 7, 2019

Great, sounds good enough for now @ahgittin, but we should keep an eye on it just in case.

@tbouron tbouron merged commit 49c5f56 into apache:master Mar 7, 2019
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.

2 participants