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

In fastboot sandbox the URL global object is not WHATWG URL #816

Open
nag5000 opened this issue Mar 19, 2021 · 10 comments
Open

In fastboot sandbox the URL global object is not WHATWG URL #816

nag5000 opened this issue Mar 19, 2021 · 10 comments

Comments

@nag5000
Copy link

nag5000 commented Mar 19, 2021

Since Node.js 10 the URL is available as global variable, and it is WHATWG URL API - "Browser-compatible URL class, implemented by following the WHATWG URL Standard".
But under fastboot I can not use new URL(...), because sandbox globals assigns URL to require('url').

let URL = require('url');
let globals = this.globals;
let sandbox = Object.assign(
{
sourceMapSupport,
console,
setTimeout,
clearTimeout,
URL,

It's very confusing because:

  1. it differs from nodejs, where globalThis.URL === require('url').URL
  2. I can't write code new URL(...) in app that will be valid for both environments - browser and fastboot.
    Instead, I have to write like this IS_FASTBOOT ? new URL.URL(...) : new URL(...) or use buildSandboxGlobals to assign URL to require('url').URL instead of default require('url').

In my opinion, the URL global variable in fastboot should be the same as for Node.js:

-   let URL = require('url');
+   let { URL } = require('url');
    let globals = this.globals;

    let sandbox = Object.assign(
      {
        sourceMapSupport,
        console,
        setTimeout,
        clearTimeout,
        URL,

Also, it make sense because support of Node < 10 has been dropped here (#695).

@rwjblue
Copy link
Member

rwjblue commented Mar 19, 2021

Ya, this is a pretty annoying bug. Thanks for reporting!

We need to figure out if this will be a breaking change though 🤔

@xg-wang
Copy link
Member

xg-wang commented Apr 7, 2021

Maybe pass require('url').URL to sandbox, and define a a getter for URL.URL with deprecation warning?

@hoIIer
Copy link
Contributor

hoIIer commented May 18, 2021

just ran into this with const _url = new URL(url)

@hoIIer
Copy link
Contributor

hoIIer commented May 18, 2021

I tried creating config/fastboot.js with the following code but it doesn't seem to work?

const { URL } = require('url');

module.exports = function() {
  return {
    buildSandboxGlobals(defaultGlobals) {
      return {...defaultGlobals, URL };
    },
  };
};

@nag5000
Copy link
Author

nag5000 commented May 18, 2021

@burritoIand Check your version of ember-cli-fastboot.
If v < 3.0 beta, use sandboxGlobals instead.

const { URL } = require('url');

module.exports = function(environment) {
  return {
    sandboxGlobals: { URL }
  };
};

@hoIIer
Copy link
Contributor

hoIIer commented May 24, 2021

@nag5000 thank you for the suggestion, I indeed am on 2.2.2
I tried using that code but I get the following error (local dev server)

Error: Could not find valid URL parsing mechanism for URL Sanitization

simonihmig added a commit to simonihmig/responsive-image that referenced this issue Aug 18, 2021
Caused by actually broken FastBoot, messing up with the `URL` global: ember-fastboot/ember-cli-fastboot#816
@simonihmig
Copy link
Contributor

Just tripped over this. In an addon, where I cannot influence the sandbox globals, I had to revert to this ugly check:

const URLConstructor: typeof URL =
        typeof URL === 'function' ? URL : (URL as { URL: typeof URL }).URL;

Would be nice to get this fixed in a stable v3!?

Maybe pass require('url').URL to sandbox, and define a a getter for URL.URL with deprecation warning?

If we want to keep compatibility with the url module and at the same time compatibility with the WHATWG API, that would mean not only exposing URL.URL, but also all the other module exports, right? This is how the export currently looks like when logged:

{
  Url: [Function: Url],
  parse: [Function: urlParse],
  resolve: [Function: urlResolve],
  resolveObject: [Function: urlResolveObject],
  format: [Function: urlFormat],
  URL: [class URL],
  URLSearchParams: [class URLSearchParams],
  domainToASCII: [Function: domainToASCII],
  domainToUnicode: [Function: domainToUnicode],
  pathToFileURL: [Function: pathToFileURL],
  fileURLToPath: [Function: fileURLToPath]
}

Doable, but also kinda ugly.

@hoIIer
Copy link
Contributor

hoIIer commented Aug 19, 2021

@simonihmig I've been struggling trying to get this to work in my app, even when using sandbox globals as below:

const { URL } = require('url');

module.exports = function() {
  return {
    buildSandboxGlobals(globals) {
      return {...globals, URL };
    },
  };
};

My app code looks like:

    const url = new URL(<url>)

edit 1

I got the above to work when using the latest 3.X beta version of this library

edit 2

Testing using ember-cli-fastboot-testing fails despite identical configuration (different lib but somewhat related so leaving here)

const { URL } = require('url');

module.exports = {
  buildSandboxGlobals(globals) {
    return {...globals, URL };
  },
};

@ryanflowers
Copy link

Bumping my version of ember-source to 3.22 solved this issue for me. The code throwing the exception is not gone, no other changes needed.

@runspired
Copy link
Contributor

we are dropping official support for fastboot from ember-data's test suite due to this issue as you cannot remove ember-fetch (not compatible with embroider-optimized / ember/ember-data 5.0) and also use ember-data due to the inability to use or expose globals like AbortController.

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

7 participants