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

Only deepClone when without prototype extensions #1107

Merged
merged 2 commits into from
Dec 18, 2019
Merged

Conversation

chancancode
Copy link
Member

@chancancode chancancode commented Dec 18, 2019

This adds a non-trivial amount of cost for each message (especially the complex ones like the view tree) that is not necessary in most apps.

Split from #1088. I'll rebase that PR once this lands.

@chancancode chancancode changed the title Only deepClone when w/o prototype extensions Only deepClone when without prototype extensions Dec 18, 2019
@wycats wycats force-pushed the no-deep-clone branch 2 times, most recently from d877766 to 9a20539 Compare December 18, 2019 07:20
This adds a non-trivial amount of cost for each message (especially
the complex ones like the view tree) that is not necessary in most
apps.
let deepClone;

if (Ember.ENV.EXTEND_PROTOTYPES.Array) {
deepClone = function deepClone(item) {
Copy link
Member

Choose a reason for hiding this comment

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

Just wanted to confirm, this is so that we can send the cloned item through the postmessage right? I ask because I want to confirm that we don't actually need to worry about deepClone being used just to be able to mutate the value...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. As far as. I can tell this is related to structure cloning. It’s done as one of the last steps before passing to chrome’s postMessage.

@chancancode chancancode merged commit 4a5de94 into master Dec 18, 2019
@chancancode chancancode deleted the no-deep-clone branch December 18, 2019 19:03
@chancancode
Copy link
Member Author

Oops! Forgot to squash and merge!

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.

None yet

3 participants