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

PageSnapshot: reduce cloning loop iterations #669

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

seanpdoyle
Copy link
Contributor

Follow-up to #666

Following a suggestion provided by review of #666, this commit
remove unnecessary loops when cloning a page's form controls to be
cached by a snapshot.

Follow-up to hotwired#666

Following [a suggestion][] provided by review of hotwired#666, this commit
remove unnecessary loops when cloning a page's form controls to be
cached by a snapshot.

[a suggestion]: hotwired#666 (comment)
}
const clone = clonedSelectElements[index]
for (const option of clone.selectedOptions) option.selected = false
for (const option of source.selectedOptions) clone.options[option.index].selected = true
Copy link
Contributor

@tleish tleish Aug 5, 2022

Choose a reason for hiding this comment

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

Yes, this change is also significantly faster than the change I suggested in the previous PR.

Overall in the HTML page I tested:

  • original: 0.5ms to 1.0ms
  • updated: 0.07ms to 0.1ms

🚀

@dhh dhh merged commit c4e0aba into hotwired:main Aug 8, 2022
@seanpdoyle seanpdoyle deleted the page-snapshot-select-loops branch August 8, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants