gatsby new: url-parse URL leaking into global scope

[ukch]

Description

I work a lot with the URL interface. Recently, I noticed my site was not working properly after some updates, and I have narrowed it down to the following:

• The url-parse module provides a replacement URL with a slightly different interface to the official whatwg version.
• Gatsby, in the latest version, is including the url-parse version in the global object (a.k.a. window)

Steps to reproduce

• Create a new project using gatsby new
• Replace the render function in src/pages/index.js with the following:

const IndexPage = () => {
    let u = new URL("http://example.com/foo.html");
    u.search = "bar=baz";
    return <h1>{u.toString()}</h1>;
}

• Run gatsby develop

Expected result

The string http://example.com/foo.html?bar=baz should be displayed inside an h1.

c.f. what happens when you paste the following code directly into JSBin:

var u = new URL("http://example.com/foo.html");
u.search = "bar=baz";
document.write(u.toString());

Actual result

The displayed URL is missing the search string. Inspecting the URL function from the browser devtools shows the url-parse version instead of the browser-built-in version.

Environment

  System:
    OS: Linux 4.16 Debian GNU/Linux buster/sid undefined
    CPU: (4) x64 Intel(R) Core(TM) i5-2430M CPU @ 2.40GHz
    Shell: 5.6.2 - /usr/bin/zsh
  Binaries:
    Node: 9.2.1 - /usr/local/nvm/versions/node/v9.2.1/bin/node
    npm: 6.5.0 - /usr/local/nvm/versions/node/v9.2.1/bin/npm
  Languages:
    Python: 2.7.15+ - /usr/bin/python
  Browsers:
    Chrome: 71.0.3578.98
    Firefox: 64.0
  npmPackages:
    gatsby: ^2.0.76 => 2.0.76 
    gatsby-image: ^2.0.20 => 2.0.25 
    gatsby-plugin-manifest: ^2.0.9 => 2.0.12 
    gatsby-plugin-offline: ^2.0.16 => 2.0.20 
    gatsby-plugin-react-helmet: ^3.0.2 => 3.0.5 
    gatsby-plugin-sharp: ^2.0.14 => 2.0.16 
    gatsby-source-filesystem: ^2.0.8 => 2.0.12 
    gatsby-transformer-sharp: ^2.1.8 => 2.1.9 
  npmGlobalPackages:
    gatsby-cli: 2.4.8

[DSchau]

Hmm... so I can't really replicate this! Are you able to provide a reproduction?

As best as I can tell, it does look like url-parse is a dependency of webpack-dev-server, but I wouldn't think that would make its way into your code.

➜ yarn why url-parse
yarn why v1.12.3
[1/4] 🤔  Why do we have the module "url-parse"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "[email protected]"
info Reasons this module exists
   - "gatsby#react-dev-utils#sockjs-client" depends on it
   - Hoisted from "gatsby#react-dev-utils#sockjs-client#url-parse"
   - Hoisted from "gatsby#webpack-dev-server#sockjs-client#url-parse"
   - Hoisted from "gatsby#react-dev-utils#sockjs-client#eventsource#original#url-parse"
info Disk size without dependencies: "64KB"
info Disk size with unique dependencies: "108KB"
info Disk size with transitive dependencies: "108KB"
info Number of shared dependencies: 2
✨  Done in 0.60s.

Additionally, here's a screenshot of the (working?) example you provided.

Please let me know if I missed anything--but a reproduction on your end would be very helpful here!

[ukch]

@DSchau
Oh, that is strange. I have zipped up my version of the starter which you can download from here. Please let me know if you can see the bug on my version.

[DSchau]

@ukch yeah - I still unfortunately can't reproduce it from my local environment, but I think there are other issues regardless, specifically:

• Using a global like you're using means that URL must exist in the Node.js environment, which doesn't seem to be the case in Linux (see these deploy logs)

  • I think if you run npm run build it will fail similarly

  • My guess is that the URL implementation in develop mode in your Linux browser is different than on my OS

  • This also means this build will fail out with something like the below in Node 8 and below

    2:23:47 PM: See our docs page on debugging HTML builds for help https://gatsby.app/debug-html
    2:23:47 PM:    7 |
    2:23:47 PM:    8 | const IndexPage = () => {
    2:23:47 PM: >  9 |     let u = new URL("http://example.com/foo.html");
    2:23:47 PM:      |             ^
    2:23:47 PM:   10 |     u.search = "bar=baz";
    2:23:47 PM:   11 |     return <h1>{u.toString()}</h1>;
    2:23:47 PM:   12 | }
    2:23:47 PM: 
    2:23:47 PM:   WebpackError: ReferenceError: URL is not defined
    

• If you want to refer to the browser variant of URL, you will need to tie into componentDidMount and then check for the existence of window.URL or use a package like whatwg-url

I've fixed the example in this repo and you can see the hosted version here

Going to close this as answered, but please feel free to re-open if I can help further.

Also a note here, that I do think this can be quite confusing. We've attempted to converge on the development/build lifecycles here, so more work to do for sure--although I'm not sure there's much we can do here to guard against this!

[ukch]

@DSchau

Using a global like you're using means that URL must exist in the Node.js environment

The issue of URL not existing in the Node.JS environment is a separate issue which I am fixing in my real project by use of universal-url.

My guess is that the URL implementation in develop mode in your Linux browser is different than on my OS

That is a misdiagnosis of the issue. As I mentioned previously, this is an issue which has only recently started occurring, specifically causing the browser-default URL implementation to be replaced by url-parse. For the record, I am using Firefox as my browser, and on other non-gatsby sites the browser-default URL implementation works as expected.

Upon further testing of the issue, I can confirm that the issue is present in Firefox but not Chrome, which (assuming you are a Chrome user) explains why you were unable to reproduce it. Please can you re-open the issue as I am not able to?

[DSchau]

That is a misdiagnosis of the issue.

This actually seems supported by the info you provide--although it does seem to be browser vs. node as I asserted. You mention that it works in Firefox (but not Chrome) on your Linux environment. That behavior is not reproducible on my OS (Mac OS Mojave 10.14.2), and I tested in Firefox, Chrome, and Safari. All fail on the build step (as you note), but all worked correctly in the local development server with the query string.

For the record, I am using Firefox as my browser, and on other non-gatsby sites the browser-default URL implementation works as expected.

These non-gatsby sites aren't server-side-rendered though, correct?

Would it be possible to create a more real-world reproduction (e.g. using the universal-url implementation) that demonstrates the issue in develop mode and also builds?

[ukch]

This actually seems supported by the info you provide--although it does seem to be browser vs. node as I asserted. You mention that it works in Firefox (but not Chrome) on your Linux environment. That behavior is not reproducible on my OS (Mac OS Mojave 10.14.2), and I tested in Firefox, Chrome, and Safari. All fail on the build step (as you note), but all worked correctly in the local development server with the query string.

It's very strange that you can't reproduce the issue at all. Are you using the provided node_modules from the zip archive, or did you rebuild them?

These non-gatsby sites aren't server-side-rendered though, correct?

Correct. But as mentioned in the original bug report, window.URL itself is being overwritten. To test:

• On Firefox, open the developer tools and switch to the console tab
• Type URL into the console, and inspect the output

On a normal site (e.g. Google or Github):

• function () with no link to the source (because it is a native function that is built into the browser)

On our example Gatsby site, during developer mode:

• function Url() with a link to the source
• Clicking the source button shows us the source, and it begins with the following comment:

/**
 * The actual URL instance. Instead of returning an object we've opted-in to
 * create an actual constructor as it's much more memory efficient and
 * faster and it pleases my OCD.

Searching Google for that string shows us that it is from the url-parse package. This means that somehow, url-parse's URL implementation is being set globally and this is being leaked to the browser.

[ukch]

I'm on Firefox 64 by the way, just in case that matters.

[gatsbot]

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

Thanks for being a part of the Gatsby community! 💪💜

[gatsbot]

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

Thanks again for being part of the Gatsby community!

[ukch]

@gatsbot please re-open this issue. I am still awaiting response from @DSchau to my last set of comments.

[gatsbot]

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

Thanks again for being part of the Gatsby community!