inrupt / solid-client-authn-js

A client library for authenticating with Solid
https://solid-client-authn-js.vercel.app
Other
69 stars 42 forks source link

TypeError: superCtor.prototype is not an object or null (node.js built in modules issues when building browser version with SnowPack) #1099

Closed joepio closed 3 years ago

joepio commented 3 years ago

First of all: thanks for creating and sharing this :)

And warning: this next issue might be a snowpack issue.

I've tried the JS example from the sandbox folder, and it works wonderfully in CodeSandbox. But if I try to run the same code in a SnowPack Typescript project, I first had to run with the --polyfillNode option (which kind of suprised me, since @inrupt/solid-client-authn-browser is a browser library, right?).

[snowpack] node_modules/@inrupt/solid-client-authn-browser/dist/SessionManager.js
   Module "events" (Node.js built-in) is not available in the browser. Run Snowpack with --polyfill-node to fix.
[snowpack] node_modules/@inrupt/solid-client-authn-browser/dist/SessionManager.js
   Module "events" (Node.js built-in) is not available in the browser. Run Snowpack with --polyfill-node to fix.
[snowpack] node_modules/@inrupt/solid-client-authn-browser/dist/Session.js
... etc

So I enable polyfilling node, and I get this:

TypeError: superCtor.prototype is not an object or null

From:
http://10.0.1.139:8080/_snowpack/pkg/@inrupt/solid-client-authn-browser.js [:61967:29]
inherits@http://10.0.1.139:8080/_snowpack/pkg/@inrupt/solid-client-authn-browser.js:61967:29
@http://10.0.1.139:8080/_snowpack/pkg/@inrupt/solid-client-authn-browser.js:64510:21

This is the block that's throwing:

if (typeof Object.create === 'function'){
  inherits = function inherits(ctor, superCtor) {
    // implementation from standard node.js 'util' module
    ctor.super_ = superCtor;
    ctor.prototype = Object.create(superCtor.prototype, {
      constructor: {
        value: ctor,
        enumerable: false,
        writable: true,
        configurable: true
      }
    });
  };
} else {
  inherits = function inherits(ctor, superCtor) {
    ctor.super_ = superCtor;
    var TempCtor = function () {};
    TempCtor.prototype = superCtor.prototype;
    ctor.prototype = new TempCtor();
    ctor.prototype.constructor = ctor;
  };
}

And that's a node.util function. Paired with the aforementioned required usage of polyfillNOde, I suspect this is where things go wrong.

When trying to find out what superCtor is:

Screenshot 2021-02-23 at 18 22 17

Is is weird that the browser plugin is using node modules? Or is that normal, and is there an issue with the snowpack node polyfiller?

Anyway, I didn't really know where to look next, so I created this issue.

Search terms you've used

superCtor.prototype, superCtor, util.inherits

Impacted package

Which packages do you think might be impacted by the bug ?

$ npx envinfo --system --npmPackages --binaries --npmGlobalPackages --browsers
System:
    OS: macOS 11.2.1
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 172.29 MB / 16.00 GB
    Shell: 3.1.2 - /usr/local/bin/fish
  Binaries:
    Node: 15.3.0 - /var/folders/dc/qlq7mcj91dvg1833drz3xmy40000gn/T/fnm_multishell_50694_1614097884902/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.0.14 - /var/folders/dc/qlq7mcj91dvg1833drz3xmy40000gn/T/fnm_multishell_50694_1614097884902/bin/npm
  Browsers:
    Chrome: 88.0.4324.182
    Firefox: 85.0.2
    Safari: 14.0.3
  npmPackages:
    @esm-bundle/chai: 4.1.5 => 4.1.5 
    @inrupt/solid-client-authn-browser: ^1.6.0 => 1.6.0 
    @snowpack/plugin-dotenv: ^2.0.5 => 2.0.5 
    @snowpack/plugin-react-refresh: ^2.4.0 => 2.4.0 
    @snowpack/plugin-run-script: ^2.3.0 => 2.3.0 
    @snowpack/plugin-typescript: ^1.2.1 => 1.2.1 
    @snowpack/web-test-runner-plugin: 0.2.1 => 0.2.1 
    @testing-library/react: 11.2.5 => 11.2.5 
    @types/json-stable-stringify: ^1.0.32 => 1.0.32 
    @types/react: ^17.0.1 => 17.0.1 
    @types/react-dom: ^17.0.0 => 17.0.0 
    @types/react-router-dom: ^5.1.7 => 5.1.7 
    @types/styled-components: ^5.1.7 => 5.1.7 
    @typescript-eslint/eslint-plugin: ^4.14.2 => 4.15.2 
    @typescript-eslint/parser: ^4.14.2 => 4.15.2 
    @web/test-runner: ^0.12.7 => 0.12.15 
    @web/test-runner-puppeteer: ^0.9.3 => 0.9.3 
    chai: ^4.2.0 => 4.3.0 
    eslint: ^7.19.0 => 7.20.0 
    eslint-config-prettier: ^7.2.0 => 7.2.0 
    eslint-plugin-prettier: ^3.3.1 => 3.3.1 
    eslint-plugin-react: ^7.22.0 => 7.22.0 
    eslint-plugin-react-hooks: ^4.2.0 => 4.2.0 
    eslint-watch: ^7.0.0 => 7.0.0 
    husky: ^4.3.8 => 4.3.8 
    lint-staged: ^10.5.3 => 10.5.4 
    polished: ^4.1.0 => 4.1.1 
    prettier: 2.2.1 => 2.2.1 
    prettier-plugin-jsdoc: ^0.3.8 => 0.3.12 
    pretty-quick: ^3.1.0 => 3.1.0 
    react: ^17.0.1 => 17.0.1 
    react-dom: ^17.0.1 => 17.0.1 
    react-router-dom: ^5.2.0 => 5.2.0 
    snowpack: ^3.0.1 => 3.0.11 
    styled-components: ^5.2.1 => 5.2.1 
    typescript: ^4.1.3 => 4.1.5 
  npmGlobalPackages:
    npm: 7.0.14
NSeydoux commented 3 years ago

Hi @joepio , thanks for reporting this, and sorry for the delayed response !

I'm not familiar with Snowpack at all, so I'm really not sure how relevant my suggestions will be here. There are multiple dependencies on NodeJS modules that need to be polyfilled for solid-client-authn-browser to run, in particular related to cryptography. Part of that is discussed in https://github.com/inrupt/solid-client-authn-js/pull/796, but basically we want to switch to a library for both solid-client-authn-browser and solid-client-authn-node, and we'll only be able to migrate the latter in April, after NodeJS 10 is no longer LTS.

However, it looks like the issue you're having is with the event module, which is used by Session in particular. So far, we did not have to polyfill it manually, which might mean that Webpack has been doing it implicitly and has hidden the problem until now. Unfortunately, that means I don't have an obvious solution, of a polyfill module to recommend that would fix the issue. I'll make sure we investigate this, and I'll update the issue as we make progress.

I'm sorry I don't have a more satisfactory answer!

joepio commented 3 years ago

Thanks for the reply and the help @NSeydoux!

I think for the the easiest fix is to switch to webpack for now, or are you saying that you'll remove the dependency on the Event module too in April?

NSeydoux commented 3 years ago

If using webpack is a viable solution for you, I think that would be the easiest course of action for now. The event module isn't one of the Node builtins that we'll no longer depend on in April, since it is part of the API it's likely to stick around.

However, I'll experiment adding an explicit dependency on https://www.npmjs.com/package/events, which is a browser module providing the events API. We wouldn't rely on webpack's behaviour in that regard, so maybe that would fix your issue.

Vinnl commented 3 years ago

We just released version 1.9.0 which contains an updated version of jose that no longer relies on native Node modules. The events package is still in there, but as far as I'm aware that won't cause a problem if you pass the --polyfillNode parameter, so as far as I know it should now at least by usable with Snowpack. We'll look at removing the dependency on Node's events module later.

Since you mentioned switching to Webpack, I'm assuming you're not going to be checking whether Snowpack works now, but if you do encounter further issues, feel free to reopen or to open a new issue. Cheers!