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

document URL parsing #1693

Closed
PauloASilva opened this issue Dec 28, 2016 · 8 comments
Closed

document URL parsing #1693

PauloASilva opened this issue Dec 28, 2016 · 8 comments

Comments

@PauloASilva
Copy link

Hi,
I did notice a miss behavior on URL parsing when using Jest (closed issue) which should be bug in or missing feature of jsdom.

I would be glad to help you further but I will need some, perhaps, some guidance.

From the issue I did open for Jest

What is the current behavior?

function parseURL (url) {
    var parser = document.createElement('a');

    parser.setAttribute('href', url);

    return {
        protocol: parser.protocol,
        username: parser.username,
        password: parser.password,
        host: parser.host,
        hostname: parser.hostname,
        port: parser.port,
        pathname: parser.pathname,
        search: parser.search,
        hash: parser.hash
    };
}
Object.defineProperty(window.location, 'href', {
    writable: true,
    value: 'http://github.com'
});

parseURL('//somehost.com');
// {
//     "hash": "", 
//     "host": "", 
//     "hostname": "", 
//     "password": "", 
//     "pathname": "", 
//     "port": "", 
//     "protocol": ":", 
//     "search": "", 
//     "username": ""
// }

This is the Firefox and Google Chrome behavior when the source code runs on a page whose document.location is about:blank what is not expected as the window.location.href property is defined by (Issue#890)

Object.defineProperty(window.location, 'href', {
    writable: true,
    value: 'http://github.com'
});

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

What is the expected behavior?

parseURL('//somehost.com');
// {
//     protocol: "https:",
//     username: "",
//     password: "",
//     host: "somehost.com",
//     hostname: "somehost.com",
//     port: "",
//     pathname: "/",
//     search: "",
//     hash: ""
// }

This is the Firefox and Google Chrome behavior when the source code run on a page whose document.location is other than about:blank.


This happened with jsdom v9.8.3 (as a Jest dependency).

Regards,
Paulo

@PauloASilva
Copy link
Author

I was digging and I think that there's no bug about parsing the URL as soon as the jsdom envionment is properly setup1.

As I am using jest, one possibility is setting the URL on package.json file, but it would be nicer to have access to the jsdom instance, so that the location could be manipulated before each test.

Does jsdom's window have API to do such changes?
I read the "Stubbing/patching window.location.href in JSDom 8?" issue and I understand your point in there about not modifying the standard API, but what about the window.location setter?
Couldn't it do the trick?

Regards,
Paulo

@domenic
Copy link
Member

domenic commented Dec 28, 2016

I'd suggest using the Jest API, since that ensures it's consistently set up the right way from the beginning. But there's also https://github.com/tmpvar/jsdom#changing-the-url-of-an-existing-jsdom-window-instance

@PauloASilva
Copy link
Author

Hi @domenic,
Jest does not expose the jsdom instance on the global scope and it only allows setting the URL on package.json file. So there's no way to call jsdom.changeURL() at runtime.

On my use case I need to change the URL per it().

I think we can improve jest+jsdom exposing the jsdom instance on global scope so that the changeURL() method can be called everywhere at any time, but I think it would also be great to have the changeURL() behavior with the window.location setter.
There is some constraint which should avoid this implementation?

@domenic
Copy link
Member

domenic commented Dec 28, 2016

We don't add nonstandard properties like changeURL to standard objects like window.location.

@PauloASilva
Copy link
Author

I understand but window.location is a standard property, using the property setter, we can accomplish the same behavior without augmenting the API

@domenic
Copy link
Member

domenic commented Dec 28, 2016

No, you can't; window.location's setter performs a navigation, which is an entirely separate piece of functionality which jsdom does not yet support (and would require a lot of work to add).

@PauloASilva
Copy link
Author

I do not agree that they are "an entirely separate piece of functionality" as when implementing the navigation part you will have to set the URL. I would say that right now it would be quite easy to make a partial implementation, just adding a setter with jsdom.changeURL() behavior.

What do you think?

I am checking with the jest guys whether or not the jsdom instance should be exposed on the global scope/environment (issue).

@domenic
Copy link
Member

domenic commented Dec 28, 2016

No, we don't have any intention of implementing a fake navigation that only changes the URL. The most important parts of navigation are things like replacing every object (Window, Document, etc.) and until we can implement that, it's better to just output a not-implemented warning.

In the end it sounds like you have a problem not with jsdom, but with jest. jsdom already has a solution to your problem, with jsdom.changeURL. So I am going to close this issue.

@domenic domenic closed this as completed Dec 28, 2016
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

No branches or pull requests

2 participants