RubenVerborgh / Solid-React-Components

Core React components for building your own Solid components and apps
https://rubenverborgh.github.io/Solid-React-Components/
MIT License
118 stars 21 forks source link

Packing issue with `solid-auth-fetcher` #32

Open NSeydoux opened 4 years ago

NSeydoux commented 4 years ago

When using a fork of the demo app updated to use solid-auth-fetcher instead of solid-auth-client (available here ), the demo app only works if webpack isn't configured to exclude @solid/query-ldflex. Otherwise, the crypto object isn't resolved properly (?), and an error a.createSign is not a function is thrown.

The confusing part is that the error only manifests when logged in.

How to reproduce

  1. Clone https://github.com/inrupt/react-components/tree/feature/experimental-auth-upgrade

  2. npm ci

  3. npm run start

  4. Go to http://localhost:8080

  5. There should be no problem yet

  6. Create an account on https://dev.inrupt.net (there is a certificate issue with the created domain, it should be resolved soon)

  7. Log in to this account from http://localhost:8080

  8. The error will appear where the user name should be displayed, and if you refresh the page, the same error will be shown instead of Ruben's name and home page. If you log out, and refresh again, the error disappears. Screencast.

  9. Stop the app

  10. In webpack/webpack.common.config.js, comment out line 24 ('@solid/query-ldflex': ['solid', 'data'],)

  11. npm run start

  12. Log in again

  13. Now, the username is properly displayed. There might be a network error on Ruben's profile, but that's an unrelated, known issue.

Version information

Additional information

RubenVerborgh commented 4 years ago

Couple of relevant clues below.

So, very long story short: my guess is that something is using certain crypto functionality that has been webpacked away.

Note in general that the webpack substitutions were very specifically tailored to solid-auth-client, and they might simply not be valid for solid-auth-fetcher.

How to resolve this: start by not making any webpack substitutions, and gradually add them until something breaks, then you've found the culprit.

This should give you some clues for detective work; what I'd still like to know is which library makes the createSign call.

RubenVerborgh commented 4 years ago

Oh and I bet it's this (deliberately incomplete) shim biting us: https://github.com/solid/query-ldflex/blob/v2.11.1/browser/crypto.js

Note that this file is shimming the Node.js crypto library, which is different from the window.crypto library.

I hope that solid-auth-fetcher is written using (a shim for) window.crypto (@jaxoncreed?), since browser shims of Node.js crypto tend to be quite big and thus undesired, whereas with Node.js shims of browser window.crypto, the size is much less of an issue.

RubenVerborgh commented 4 years ago

Oh and I bet it's this (deliberately incomplete) shim biting us

It most definitely is https://github.com/solid/query-ldflex/blob/v2.11.1/webpack/webpack.common.config.js#L30-L31

because when you do

comment out line 24 ('@solid/query-ldflex': ['solid', 'data']

you bypass that substitution.

I really hope we don't need to include all of Node crypto in the browser, because that's huge.

NSeydoux commented 4 years ago

You're right, commenting out that exact substitution resolves the issue. I'll see what can be done to shim the required functions.

RubenVerborgh commented 4 years ago

@NSeydoux That part is optional though; it's only for size reduction. Probably non-trivial to fix.