Proposal: split out the auth components

[Vinnl]

Motivation

Users of @solid/react might only be interested in using the authentication-related components (useWebId, <LoggedIn>, etc), and use a library like rdflib to interact with RDF. It's a bit wasteful to also send them LDFlex and Communica.

Proposal

Extract the auth-related components into e.g. a @solid/react-auth package, and making @solid/react depend on that package.

[RubenVerborgh]

wasteful

It's an ES6 module, so tree shaking should work?

[Vinnl]

Two thoughts there:

1. Unfortunately there are many things that can break tree-shaking, I believe. It doesn't tree-shake for me - shall I report a new bug for that?
2. While 1. is certainly the main problem, there's still a downside to having those two libraries and all their dependencies shipped to the developer's machine even when they're not sent to the client, such as the higher risk of malicious postinstall scripts, and increased disk usage.

But yes, tree-shaking would remove the main pain point. Unfortunately, I've hardly ever seen it working reliably.

[RubenVerborgh]

1. It doesn't tree-shake for me - shall I report a new bug for that?

Yes please, let's fix tree shaking first. Can you include a reproducible example (or link to a minimal repo)?

[RubenVerborgh]

Update: tree shaking can indeed not work the way things are at the moment. So no real need for an example yet.

[RubenVerborgh]

ca154add36c9f7fb06fcc4b68bb9c0a41722ee40 should provide you with the necessary; can you check?

[Vinnl]

@RubenVerborgh Could you provide some guidance on how to test that? I've currently run:

$ yarn add https://github.com/solid/react-components.git#ca154add36c9f7fb06fcc4b68bb9c0a41722ee40
$ cd node_modules/@solid/react
$ npm install
$ npm run build

But that final step fails with a bunch of errors to the gist of:

ERROR in ./src/components/withWebId.jsx 10:11
Module parse failed: Unexpected token (10:11)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
|  */
| export default higherOrderComponent('WithWebId', Component =>
>   props => <Component {...props} webId={useWebId()} />);
| 
 @ ./src/index.js 9:0-47 34:0-68:2

[RubenVerborgh]

Confirmed that webpack acts up when installed in a node_modules subfolder because of a configuration error. You can still test with npm run build:module.

[Vinnl]

I tested it in a new Create React App setup just now using npm link, but am now hitting the following error:

Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:

1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.

I only added

import { LoggedOut } from '@solid/react';

and

          <LoggedOut>Test</LoggedOut>

(Btw, this feeds back into our earlier discussion on why npm installing (or yarn adding) from GitHub isn't a replacement for installing from npm ;) )

[RubenVerborgh]

You are likely hitting You might have more than one copy of React in the same app.

why npm installing (or yarn adding) from GitHub isn't a replacement for installing from npm

Definitely; I have found React to be very troublesome in these cases. It behaves weirdly with npm link.

What I usually do there is remove the node_modules folder from the linked repository, such that i is ignored. It's a bit of fiddling.

Let me know if you can work it out; if not, I'll release a beta.

[Vinnl]

@RubenVerborgh Thanks, removing node_modules after linking did fix it.

And yes, tree shaking works now, thanks!

[RubenVerborgh]

Super, thanks for testing. I imagine the gains in size to be quite significant 🙂

[RubenVerborgh]

Published as v1.8.0.