Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support route conditional fragments returning an array #915

Closed
wants to merge 1 commit into from

Conversation

xuorig
Copy link
Contributor

@xuorig xuorig commented Mar 6, 2016

Gave #896 a try.

modifies createNode and getChildren to possibly return an array of nodes when a route conditional fragment reference returns an array.

Let me know what you think, It's seems weird to have createNode possibly return an Array but I'm not sure how it could be done without it!

if (node && node.isIncluded()) {
nextChildren.push(node);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that [].concat(...) will be slower than a manual check, especially since most cases will return a single value not an array. Let's explicitly check for array/scalar similar to what you did below.

@xuorig xuorig force-pushed the array-route-cond-fragments branch from 00d83a6 to 8c45667 Compare April 16, 2016 17:26
@xuorig
Copy link
Contributor Author

xuorig commented Apr 16, 2016

Switched to manual check!

@xuorig xuorig force-pushed the array-route-cond-fragments branch from 8c45667 to a86d712 Compare April 21, 2016 01:25
@wincent
Copy link
Contributor

wincent commented Jun 24, 2016

Sorry for leaving this one unattended for so long, @xuorig. Going to pull it in and see if it applies, tests pass etc.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants