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 the global structuredClone() method #3459

Closed
wants to merge 1 commit into from
Closed

Add the global structuredClone() method #3459

wants to merge 1 commit into from

Conversation

realtimetodie
Copy link

Adds the global structuredClone() method to the Window scope. The method was introduced in Node.js version 17.0.0 (release notes). The available polyfills of the structuredClone() method for Node.js are considered to be incomplete in comparison to the web browser JavaScript runtime implementations.

Example - Error message when the structuredClone() method is not defined in the global scope of Node.js (Node.js < 17)

ReferenceError: structuredClone is not defined

Users can provide their own polyfill of the structuredClone() method (Node.js < 17)

global.structuredClone = (value, options) => ...

References

Resolves #3363

@realtimetodie
Copy link
Author

@domenic

@domenic
Copy link
Member

domenic commented Nov 10, 2022

This does not implement the specification linked, or have any tests.

@realtimetodie
Copy link
Author

Excuse me, but the structuredClone() method is a global object in the Node.js runtime, and the implementation simply reflects this method into the VM context. How does this "not implement the specification linked"?

@domenic
Copy link
Member

domenic commented Nov 11, 2022

Because Node's structured clone operates on Node objects, not on jsdom (web) objects.

@realtimetodie
Copy link
Author

Alright, fair enough. So, according to the structured clone algorithm, the following JSDOM Web/API types need to be covered differently, otherwise their non-enumerable properties, property accessors, and object prototypes are not preserved and they will be cloned as plain JavaScript objects.

I am looking into this and will add some tests as requested.

@domenic
Copy link
Member

domenic commented Nov 11, 2022

You shouldn't need to add too many tests. They should already exist in web platform tests. See CONTRIBUTING.md. The main goal here is to enable https://github.com/web-platform-tests/wpt/blob/master/html/webappapis/structured-clone/structured-clone.any.js ; if you can get that passing, the PR is probably ready.

@acywatson
Copy link

acywatson commented Apr 9, 2023

@domenic do you have any ideas on how I could move this forward? It seems like core-js has an implementation that follows the spec, AFAICT. Should we copy that code over, modifying it as necessary? It's unfortunate to have it in two places, but I'm not sure if you're interested in taking a dependency on core-js for this. Either way, I see no real reason to re-implement this from scratch in js-dom.

@domenic
Copy link
Member

domenic commented Apr 9, 2023

I think I gave pretty clear instructions above, about how to proceed. If you want to see someone following them, check out #3521 .

@acywatson
Copy link

I think I gave pretty clear instructions above, about how to proceed. If you want to see someone following them, check out #3521 .

I'll have to respectfully disagree - if it was clear, then I wouldn't be asking a clarifying question about it :). I think that's a reasonable thing to do before starting work on something like this.

Great to see someone else is working on this, though and I appreciate the response.

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.

[Bug]: structuredClone is not defined
3 participants