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

Add enhancement for discusssion #3293 #3617

Closed
wants to merge 2 commits into from

Conversation

puckowski
Copy link

* Add watch enhancement for discussion alpinejs#3293.
@puckowski
Copy link
Author

puckowski commented Jun 9, 2023

CodePen with cdn.min.js from main_3293 forked from https://codepen.io/dinyo/pen/eYVaKgj?editors=1111 here: https://codepen.io/puckowski/pen/poQJzzv?editors=1111

@ekwoka
Copy link
Contributor

ekwoka commented Jun 10, 2023

I think it could be beneficial to check for and prefer structuredClone.

Maybe set this up more as a polyfill type thing.

At script init do something like const deepClone = globalThis.structuredClone ?? customClone

So we can prefer the future :)

@puckowski
Copy link
Author

When structuredClone encounters a proxy object, it throws a DataCloneError and fails to clone the object.

I ran into DataCloneError issues when I was using structuredClone for #3046

This presented issues as there is no great way to tell if an object is proxied in JavaScript, thus I had to support a simple shallow clone implementation along with structuredClone. There were concerns about the number of lines added with 3046.

Plus, depending on how many browser versions the Alpine user wishes to support, they may need to polyfill structuredClone (https://caniuse.com/?search=structuredClone). Or, we add the structuredClone polyfill to Alpine for all users at the expense of bundle size. Again, that may not be ideal in terms of number of lines added.

I think 3617 as is allows users of Alpine to reliably get the old and new value with $watch with few lines of code added while passing tests.

@calebporzio
Copy link
Collaborator

Thanks for the PR - I'm wondering if we can just use the simple JSON.parse(JSON.stringify(...)) clone method

It obv won't keep functions around, but I think it's ok to have that as a limitation when accessing "old" right?

@ekwoka
Copy link
Contributor

ekwoka commented Sep 11, 2023

Is there risk of circular references?

@calebporzio
Copy link
Collaborator

Thanks for contributing!

In order to keep this project as focused and maintainable as possible, I'm going to close this pull request.

This isn't "no", it's just "no for now".

If you disagree, or there's further demand from the community, please re-open this PR and we'll reconsider!

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.

None yet

3 participants