Editor: Replace deep clone dependency with native structured clone #11799
Conversation
I searched the code base ... I didn't turn up any instances. |
|
Just checking this is not related web-stories-wp/packages/story-editor/src/utils/getUniquePresets.js Lines 16 to 19 in e00e50d |
Not related, here we would need something native for deep comparison. |
|
Regarding the #11505 (comment) @swissspidy you commented on May 18 and the second Safari version with |
|
I think the polyfill is automatic from the browser config for whatever browsers need it
See: #11505 (comment)
|
|
I think what Marcin is saying is that with our current browserslist config the polyfill would not be included anymore, since the addition to browsers is rather new but there have been >2 new releases since then, yet not all users might be on these latest two versions. So in accordance with our browser support policy, the question is whether we'd want/need to force our polyfill to be included so that our editor still works for users on Safari 15.3 or older. So the ask is to check analytics to see how our Safari usage looks like to inform such a decision. I'll happily check our analytics. From what I can see at first glance, the polyfill should still be included because of the "last 2 Opera versions" directive, and structuredClone was only added in the latest Opera version v84. I haven't checked the built code though to confirm. |
|
Size Change: +9.06 kB (0%) Total Size: 2.65 MB
ℹ️ View Unchanged
|
|
Plugin builds for d244fee are ready 🛎️!
|
Opera should be fine, I have v86 and
|
|
So it looks like ~20% of Safari users are on version ≤ 15.3, so it would be good if we could include the polyfill for now. We should be able to achieve this by adding Of course needs verification that the polyfill is indeed included as expected. |
|
^ Can you all confirm that the polyfill is included with this change? Needs testing for sure. |
I'm testing that and something is wrong here, even |
|
I tested it on multiple browsers e.g. Chrome 97. The polyfill is just not included no matter what I try, I also tested |
|
Does changing this line: web-stories-wp/babel.config.cjs Line 35 in 5efe289 to I just read in the docs that apparently When I do this and run With |
It works! 🎉
- Who puts "Warning" in a grayed-out color instead of big bold red? 😄 |
…js: 3 doesn't include minor releases - credit to Pascal
Context
structuredCloneis now available in all browsers. We currently useclone-deepfor this purpose.Summary
Replaces clone-deep with structuredClone
Relevant Technical Choices
To-do
User-facing changes
Testing Instructions
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
No
Does this PR change what data or activity we track or use?
No
Does this PR have a legal-related impact?
No
Checklist
Type: XYZlabel to the PRFixes #11505