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

fix: Prevent deserialize break on safari < 15.4 #1483

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

igorbrasileiro
Copy link
Contributor

@igorbrasileiro igorbrasileiro commented Jul 19, 2023

The problem

The Array.prototype.at is only available at safari >= 15.4 caniuse
image. So, the hydration breaks in older browsers.

Preview: https://deco-sites-fashion-nfcj5n1zeycg.deno.dev/

The solution

This PR changes the .at(-1) to [array.length - 1].

@igorbrasileiro igorbrasileiro changed the title fix: Prevent deserialize breaking on iphone 12 fix: Prevent deserialize break on iphone 12 Jul 19, 2023
@igorbrasileiro igorbrasileiro changed the title fix: Prevent deserialize break on iphone 12 fix: Prevent deserialize break on safari < 15.4 Jul 19, 2023
@@ -56,7 +56,7 @@ export function deserialize(
(o, k) => k === null ? o : o[k],
v,
);
parent[refPath.at(-1)!] = target;
parent[refPath.slice(-1)[0]!] = target;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cloning the array just to get the last value seems a bit much. Let's skip the temporary array allocation here by accessing the last item directly.

Suggested change
parent[refPath.slice(-1)[0]!] = target;
parent[refPath[refParent.length - 1]!] = target;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, sure! Thank you =D

Signed-off-by: igorbrasileiro <brasileiro456@gmail.com>
Copy link
Collaborator

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a bunch for taking the time to make this PR ❤️

@marvinhagemeister marvinhagemeister merged commit 82b0f2c into denoland:main Jul 19, 2023
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