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

Browser distribution: Uncaught TypeError: Cannot read property 'split' of undefined #3934

Closed
cmorten opened this issue Jan 24, 2021 · 3 comments

Comments

@cmorten
Copy link
Contributor

cmorten commented Jan 24, 2021

Expected Behavior

BAU, or if an error case arises, a deliberate and descriptive Rollup thrown error (e.g. entry module does not exist) when using the Browser distribution of Rollup.

Actual Behavior

Executing:

import {
  rollup,
} from "https://unpkg.com/rollup@2.38.0/dist/es/rollup.browser.js";

const options = {
  input: "src/mod.ts",
};

rollup(options);

In the browser (or Deno) results in the following thrown error:

error: Uncaught TypeError: Cannot read property 'split' of undefined
    at Et (https://unpkg.com/rollup@2.38.0/dist/es/rollup.browser.js:11:65715)
    at ja (https://unpkg.com/rollup@2.38.0/dist/es/rollup.browser.js:11:325076)
    at async Qa.loadEntryModule (https://unpkg.com/rollup@2.38.0/dist/es/rollup.browser.js:11:334442)
    at async Promise.all (index 0)

Please see the reproduction (https://gist.github.com/cmorten/fdffbd9ce521b6436f5fcbce429e752d) for complementary details. Summary below:

  • Rollup's browser implementation of resolve() does not cater for no arguments being passed.

  • Rollup's module resolution code calls resolve() with no arguments in certain scenarios.

  • In NodeJS, calling resolve() with no arguments is valid and will return the CWD, but in the Rollup browser implementation results in attempting to call .split() on undefined.

  • I suspect the fix may be as simple as replacing the code at https://github.com/rollup/rollup/blob/master/browser/path.ts#L61 with

    let resolvedParts = (paths.shift() || "").split(/[/\\]/);

    but it may well not be that simple. Alternatively, removing the use of resolve() with no arguments from https://github.com/rollup/rollup/blob/master/src/utils/resolveId.ts#L31 with something like

    importer ? resolve(dirname(importer), source) : resolve(source),

    given that if the provided arguments are insufficient to form an absolute path then the behaviour of resolve() is to prepend the CWD, and therefore the inclusion of the resolve() currently is both superfluous according to specification, and causing the Rollup browser distribution to thrown an Uncaught TypeError in very simple cases.

    If, after processing all given path segments, an absolute path has not yet been generated, the current working directory is used.

    REF: https://nodejs.org/api/path.html#path_path_resolve_paths

Context: I am using the browser distribution as the core for a Deno port of Rollup in which I am having to work around this issue currently.

Happy to put in a PR if the issue stands (and haven't missed something obvious!)

@shellscape
Copy link
Contributor

Does this happen with Node LTS? I'm not entirely sure we're supporting Deno as yet.

@cmorten
Copy link
Contributor Author

cmorten commented Jan 24, 2021

No this does not happen in Node. It is specifically a problem with the Browser distribution of Rollup which afaik is officially supported (it drives the online REPL for instance).

I have updated the original description to primarily reference this is an issue with the browser distribution, it is simply a happy coincidence that the browser dist is compatible with Deno, and that is how I originally spotted the issue.

Update: to further confuse things, there is little documentation r.e. official browser support but #3012 and https://rollupjs.org/guide/en/#a-simple-example hopefully provide some context.

@lukastaegert
Copy link
Member

Resolved via #3935

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

3 participants