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

Use URL when available natively #110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

iambalaam
Copy link

@iambalaam iambalaam commented Dec 16, 2019

  • Add a meaningful entry to the changelog in the README.md.
  • You can add yourself to the list of contributors in the package.json if you
    like. If you don't do it, you won't be mentioned.

This is my first attempt to solve Cannot clone a URL in Node >= 10 #109 .


I have had to clone the property descriptors of Symbols, and I assume I should do the same for property names.

When running the following code I found that the value wasn't a primitive. This made the URL object clone to a new object, but the URLSearchParams inside it point to the same instance. (I have not assessed the performance implications of this.)

> const url = new URL('https://example.com/?a=1');
> const querySymbol = Object.getOwnPropertySymbols(url)[1] // Symbol(query)
> Object.getOwnPropertyDescriptor(url, querySymbol)

{ value: URLSearchParams { 'a' => '1' },
  writable: true,
  enumerable: true,
  configurable: true }

@iambalaam
Copy link
Author

Since raising this PR, I have questioned if both parts of the solution are needed (nativeURL and cloned property descriptor/names). I have now tested and know that cloning the property descriptor/names is enough on its own.

I am now unsure if there are any merits of adding the nativeURL, or if I should remove that part?

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.

1 participant