Skip to content

Speed up patching 5-10x#2845

Merged
chrismccord merged 20 commits intomainfrom
cm-optimize-patch
Oct 13, 2023
Merged

Speed up patching 5-10x#2845
chrismccord merged 20 commits intomainfrom
cm-optimize-patch

Conversation

@chrismccord
Copy link
Copy Markdown
Member

No description provided.

@chrismccord chrismccord requested a review from josevalim October 4, 2023 20:46
Comment thread assets/js/phoenix_live_view/dom_patch.js Outdated
Comment thread assets/js/phoenix_live_view/dom_patch.js
Comment thread assets/test/modify_root_test.js Outdated
Comment thread assets/js/phoenix_live_view/rendered.js Outdated
Comment thread assets/js/phoenix_live_view/rendered.js
Comment thread assets/js/phoenix_live_view/rendered.js Outdated
Comment thread assets/js/phoenix_live_view/rendered.js
Comment thread assets/js/phoenix_live_view/rendered.js Outdated
@jswanner
Copy link
Copy Markdown
Contributor

jswanner commented Oct 5, 2023

@chrismccord, tags with phx-remove are being reported in the console as duplicating IDs when using this branch:

utils.js:19 Multiple IDs detected: filter-modal. Ensure unique element ids.

Comment thread priv/static/phoenix_live_view.js Outdated
Comment thread priv/static/phoenix_live_view.js Outdated
Comment thread priv/static/phoenix_live_view.js Outdated
Comment thread priv/static/phoenix_live_view.js
@chrismccord chrismccord requested a review from josevalim October 11, 2023 17:39
Comment thread assets/js/phoenix_live_view/rendered.js Outdated
// experienced a change which must be a newRender, and thus we can't skip the render.
//
// Both optimization flows apply here. newRender is set based on the onlyCids optimization, and
// we track a deterministic magicId based on the cid.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👏 👏 👏

Comment thread assets/js/phoenix_live_view/rendered.js
Copy link
Copy Markdown
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

Glorious, I have added some comments. Feel free to incorporate or discard them.

@josevalim josevalim changed the title Speed up patching 5x Speed up patching 5-10x Oct 12, 2023
@bcardarella
Copy link
Copy Markdown
Contributor

Is this considered a backwards incompatible change? We're assessing it for LVN. I suspect if a LV server updates they'll need a new version of the LVN client

@josevalim
Copy link
Copy Markdown
Member

No, the server changes are minimum (1 LOC) and you can safely ignore the additional parameter.

chrismccord and others added 3 commits October 13, 2023 08:44
Co-authored-by: José Valim <jose.valim@dashbit.co>
Co-authored-by: José Valim <jose.valim@dashbit.co>
@chrismccord chrismccord merged commit 3d28c02 into main Oct 13, 2023
@chrismccord
Copy link
Copy Markdown
Member Author

❤️❤️❤️🐥🔥

@chrismccord chrismccord deleted the cm-optimize-patch branch October 13, 2023 12:48
chrismccord added a commit that referenced this pull request Oct 13, 2023
Co-authored-by: José Valim <jose.valim@dashbit.co>
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.

5 participants