solid-contrib / solid-rest

a client-side API which supports any backend for Solid requests
37 stars 9 forks source link

Reconsider handler initialization #59

Closed CxRes closed 3 years ago

CxRes commented 3 years ago

Currently, handler initialization is based on checking the window object. I presume this has to do with detecting if the environment is a browser. However, this check is a problem in nwjs/electron environments where both window and node are defined. In such cases we still want to create file handlers and can even use native localstorage of the browser. At least checks should be made for both window and process.

I am not sure of the best strategy to manage this tough and would like to hear a bit more about the design intent in automatically initializing handlers. I see that one can pass options through solid-node-client (incl. options.handlers) but these are not documented. That is why I have refrained from putting up a PR!

Somewhat unrelated but relevant to the issue, I think in the long term solid-rest should be decoupled from solid-node-client (and other providers should be decoupled as well, as this list will become unwieldingly large if this project takes off). Each type of source then should be a plugin that is invoked based on user-installation (perhaps done so automatically on first invocation of the scheme) and url scheme. In fact, this is the approach I am taking in including solid in my application. Please consider this in your future plans!

jeff-zucker commented 3 years ago

I think you have misread the code for the handlers. (It's a bit messed up because I had to retrofit some things to maintain backwards compatibility and also handle a passed in rdflib for patch.) The way it works is that if the user does not pass in one or more handlers, it checks the operating environment and pulls in the default handlers for that environment. However, if the user does pass in some handlers, it uses those and does not check the environment at all. So in Data-Kitcehn (an electron app), I pass in both the nodejs and the browser-based handlers in the call to new SolidRest() because it is capable of operating in both environments.

I am behind on documenting Solid-Rest. I need to do a lot of work there and mentioning options.handlers is a big one.

I am unclear what you mean about decoupling from solid-node-client. There is nothing in solid-rest that requires solid-node-client and solid-rest is quite capable of operating without it. And in solid-node-client (although undocumented as yet, I just put this feature in) you can pass in any fetch handler with the call to new SolidNodeClient. Additionally, rdflib currently looks for global.solidFetcher allowing users to define the fetcher at the top of their app rather than passing it in when creating a fetcher. Solid-client-authn-nodejs (if it ever comes out) also plans to look for a global fetch to allow users to override the built-in. So it seems to me that an app can easily mix and match auth/login libraries and fetch libraries. Am I missing something?

If I've misunderstood your suggestions or my response doesn't answer them, please ask again.

CxRes commented 3 years ago

The way it works is that if the user does not pass in one or more handlers, it checks the operating environment and pulls in the default handlers for that environment.

I think this could do with some improvement (since it fails in nwjs), by checking for node and process objects for instance. Just because window is undefined is no reason node.fs is defined or vice-versa. Check should be something on the lines of: Is node defined? then FS; Is localStorage defined? then localStorage else Memory.

In my case of nwjs, it goes false on typeof 'window' === undefined , then fails the try because browserFS is not a constructor (for reasons I do not understand as of now) and ultimately crashes at handler.forEach because handlers is initialized as null! I am guessing, the bug is that handlers should at least be an empty array. But we came to that only because we could not define any handler at all!

Meanwhile could you please point me to the handler invocation in Data-Kitchen. At least, I can side-step the issue that way for now.

There is nothing in solid-rest that requires solid-node-client.

Its the other way around that I have an issue with, ie solid-node-client requires solid-rest. So just to use solid-node-client (essentially the auth and fetch in it with say, solid-file-client) I have to pull in all of solid-rest. Let me put this in another way: In my ideal scenario (this is my opinion, though held strongly), solid-rest would be a thin wrapper that passes controls to different protocols and one would install solid-node-client or solid-localstorage etc. as plugins. Currently, fs: or app:/ls/ etc. come baked in with solid-rest (even if I do not want to support one of these protocols in my end product or any for that matter) which in turn come baked in when I install solid-node-client. It is a classic banana-gorilla problem!

If there is another mechanism you have in mind or currently exists that I am missing, I am all ears!!!!!

Additionally, rdflib currently...

I'll come to that in a separate issue. rdflib uses outdated deps and I have had Rollup screaming madly at me last night when I activate it. I hope we can find some other way to enable PATCH.

jeff-zucker commented 3 years ago

Solid-Node-Client does nothing but add solid-rest's fetch to whatever the underlying auth is. Currently that is solid-auth-fetcher but if and when there is a solid-client-authn-node, it will switch to that. If one doesn't need solid-rest, one can use either of those auth libraries directly without solid-node-client.

The failure in nwjs (which I had never heard of but looks interesting) is concerning. My feeling is that I should add a note in the documentation that if you are operating in a mixed environment such as nwjs or electron, you MUST supply your own handler(s) in the call to new SolidRest(), see below for an example.

I agree that solid-rest should be as light as possible, but for backward compatibility and general principles, I would like it to function out of the box for file:// requests in node. Maybe I'm wrong about how conditional requires work, but my impression is that file.js and the other plugins are only imported automatically if no other handlers are supplied.

Rdflib is in rapid development and so problems with including it may be temporary. I encourage you to raise an issue or go to the rdflib chatroom if you are having diffuculty with it.

For cases in which the user does not want the built-in plugins, passing in a handler will override them and they will not be loaded. For example, this instantiation would use localStorage.js but not file.js.

const SolidRest = require('solid-rest')                                         
const LS = require('solid-rest/src/localStorage')                               
const rest = new SolidRest({                                                    
  handlers : [new LS()]                                                         
}); 

There is a working example of doing this in a browser at solid-rest/tests/browser-test.html. It instantiates the object like so:

<script src="../bundles/browserfs.min.js"></script>
<script src="../src/browserFS.js"></script>  
<script src="../dist/browser/solid-rest.js"></script>
<script>
const rest = new SolidRest({
  handlers : [new SolidBrowserFS()]
})
...
jeff-zucker commented 3 years ago

I do not understand "install solid-node-client ... as plugins". Solid-node-client is not a plugin to solid-rest, it's the other way around. Here is the chain as I see it:

define a non-http-fetcher (solid-rest+plugins)                                  
define an http-fetcher (solid-auth-fetcher)                                     
create a hybrid fetcher that uses both (solid-node-client)                      
specify another library's fetcher as that hybrid (rdlib,solid-file-client,etc.) 
CxRes commented 3 years ago

Sorry, this post goes off topic into the plugin issue. I'll return to it in the next comment...

Maybe I'm wrong about how conditional requires work, but my impression is that file.js and the other plugins are only imported automatically if no other handlers are supplied.

That is only true if you are invoking the code from node (and then too the code exists in node_modules). Bundlers like Webpack and Rollup will pull all the requires unless you specifically ask it not to (which requires you to dig in and manually identify these require calls). I have all this code in my bundle even when I do not want to use most of these fetchers. I would like to avoid that!

I do not understand "install solid-node-client ... as plugins".

That clarifies my mental model a bit (perhaps influenced by solid-auth-cli, or my impression of it). Hopefully, I can now explain my idea better.

In my model, I would ideally like to see a fetcher manager, lets call it solid-fetch-manager wherein http-fetcher, file-fetcher, localstorage-fetcher etc. each will be a plugin. Now, I imagine 2 modes: 1) Manual mode: wherein developer will choose what fetcher they want. (It is not inconceivable if solid catches on, a developer may create an app that only stores data locally for a session using say, a local-storage fetcher (and not even use a http-fetcher). Or another protocol instead of http catches on? Or I want a desktop only app! Ultimately, it is solid that matters as the standard for data communication not the transport/storage, that it is built first on http is incidental. And solid-rest is proof of that imho!) 2) Automatic mode: where based on the protocol on the address, the fetcher-manager installs or downloads the correct fetcher from a CDN. (would work extremely well with something like deno)

Ultimately, I am trying avoid any bloat being added due presumptions made, even if it is a completely reasonable presumption like supporting http: and file: out of the box. To support these assumptions, we can simply install http: and file: fetchers anytime one installs the solid-fetch-manager.

Re: Example Just one pedantic point which drags on the plugin issue... it is generally a bad idea in my experience to call files inside a library ie solid-rest/src/localStorage (since they might get remapped/renamed at some point, I have filed such bugs before). Partially, we can fix this with the exports property in package.json, but really, I think it is a sign that they need to be in separate repos!

CxRes commented 3 years ago

NWjs (https://nwjs.io) was the original electron. One of their alumni was responsible for much of the work on electron (and it got pretty acrimonious). It has both window and node environments together. Thats why I am proposing a better check model for handler association. I can make a PR for you to review, since I think it would be a non-breaking change otherwise!!!

jeff-zucker commented 3 years ago

I am thinking of this as the interface for SolidNodeClient. In this example, the user does their own importing and merely passes things down the line.

let client  = new SolidNodeClient({                                             
  fetchers : [                                                                  
    { http : new SolidAuthFetcher() },                                          
    { file : new SolidRestFile() },                                             
    { bfs  : new SolidRestBrowserFS() },                                        
  ],                                                                            
  parser : require('rdflib'),                                                   
})                                                                              

The fetchers are optional - user can import only ones for protocols they expect and the parser is optional, only required if user wants PATCH. SolidRest would use only the pre-imported handlers and parsers passed to it, and would not directly import any of the plugins or rdflib so would not have any of them in the dependency chain.

If new SolidNodeClient() is called with no fetchers, it would default as now. This would mean that it would need to import SolidRestFile. But SolidRest would not have to import it. So if the user wants SolidRestFile out of the dependency chain, they can just use Solid-Rest directly.

How does that sound?

CxRes commented 3 years ago

How does that sound?

This sounds much closer to what I had in mind!!!!!

If new SolidNodeClient() is called with no fetchers, it would default as now. This would mean that it would need to import SolidRestFile. But SolidRest would not have to import it. So if the user wants SolidRestFile out of the dependency chain, they can just use Solid-Rest directly.

This confuses me (perhaps I am missing something about the internal design). Why would one need solid-rest anymore (except for legacy reasons) when doing this? Couldn't one design it in such a way as to...

import { SolidNodeClient } from 'solid-node-client';
import SolidAuthFetcher from 'solid-auth-fetcher';
import SolidRestFile from 'solid-rest-file';
import SolidRestBrowserFs from 'solid-rest-browser-fs';

let client  = new SolidNodeClient({                                             
  fetchers : [                                                                  
    { http : new SolidAuthFetcher() },                                          
    { file : new SolidRestFile() },                                             
    { bfs  : new SolidRestBrowserFS() },                                        
  ],                                                                            
  parser : require('rdflib'),                                                   
}) 

Am I right in presuming currently you plan to:

...
import SolidAuthFetcher from 'solid-auth-fetcher';
import SolidRestFile from 'solid-rest/src/file.js';
import SolidRestBrowserFs from 'solid-rest/src/browser-fs.js';
...

AFAICT, there is nothing except organization of repos that privlidges http fetcher over non-http fetchers? Or again am I missing something?

There is one other thing that is bothering me but I am afraid I do not have clarity of mind about the problem itself. My concern is that on local networks, such as the one I have at home, one has a login per computer (which effectively is a domain, though not officially). So, I have different logins for file://computer1/D:/path/to/file and file://computer2/path/to/another/file (file: with two slashes and not regular 3, like http://) which is different for my local computer file:///yet/another/path. My concern is that if I have to mix and match data from these fetch calls we would need to have separate clients, even though each login pertains to each individual computer. Similar issues would arise in http: when user has multiple identities but wants fetch data from different domains (though here the common login might work across domains) towards a common task. I do not know what is the best way to address this and would like more time to think this through. But I just want to put this concern out there...

jeff-zucker commented 3 years ago

Why would one need solid-rest anymore?

Because it is the glue that makes the plugins work. SolidRestFile would be the same as the current solid-rest/src/file and, like it, would need solid-rest to call it.

The local network is an interesting issue, I haven't thought about it. But there would be no login - solid-rest and solid-node-client do not have any login for local files, only for pods. I don't see why different clients would be needed. Can you do a test and see if you can do a fetch against each of the URLs you mentioned?

CxRes commented 3 years ago

Because it is the glue that makes the plugins work

I thought solid-node-client is the glue. It seems strange that you need separate glue first for non-http and then tie in http with the non-http. Should any and every protocol not be at par?

Can you do a test

I can tell you as it is it would not work without OS specific authentication (at least for windows). And I am not worried if it does not work just today, but the provision for long term!

jeff-zucker commented 3 years ago

Solid-Node-Client creates a client that calls an auth package to perform login and authenticated fetches for http requests and calls solid-rest to handle non-http requests. It only works in nodejs, not in a browser. It does not do any heavy lifting, it just creates a hybrid fetcher that performs both kinds of fetching. It does not understand or interpret either requests or responses, merely passes them on.

Solid-Rest works either in nodejs or in a browser, depending on which of its plugins is used. It gets requests for non-http resources from a client (SolidNodeClient, or directly in an app,etc.) and, depending on the protocol, hands the actual interaction to a plugin such as solid-rest/src/file, gets back a response, packages that as a Solid HTTP response and sends it back to the client.

Anything specific to a nodejs environment belongs in Solid-Node-Client or in a Solid-Rest plugin but not in Solid-Rest itself.

I do not at all understand by what you mean by "at par".

I also do not understand what part of your networking example would need to be implemented in solid-rest or how solid-rest would need to behave differently to accommodate a plugin oriented to a networked environment. It seems to me that you could send an ssh login then proceed to use solid-rest over the ssh connection.

CxRes commented 3 years ago

Loooooooong post warning...

At Par

Indulge me here... Solid is meant to be a universal linked file system, that is, it does not matter where the data is and how we access it. We give Solid an address, it gives us our data and if that data contains links, it goes back and fetches that data as well. It does not matter if that address begins with http or file etc because http, file etc are merely physical storage/transport. This we already agree upon.

Now the current architecture looks like this:

client (solid-node-client)
|- http fetcher (solid-auth-fetcher)
`- non-http fetcher (solid-rest)
    |- file fetcher
    |- ... etc.

The fact that you switch in two steps mean that you consider http to be privileged. From what I have understood, this privilege is due to the fact: 1) Solid was designed for the web first and on top of http. 2) On the web authentication is a must (for editing).

This special status to http is an implicit assumption in how things stand. And this assumption violates the generality that Solid strives for or should strive for. This in the future will become a source of complexity (in that, you will forever maintain atleast two switching layers).

A different way to design would be:

client 
|- http fetcher
|- file fetcher
|- ... etc.

Apart from maintaining a single layer, it also means that other protocols will be first class citizens. Think about this: if a computer in the future boots into Solid os, the first thing you would need to do is authenticate user on the file: protocol. You will far more often access file: over other protocols. Now, for example, is it not inefficient to first decide http or non-http and then go to file.

I see that there is a browser/node dichotomy, which creates the need for separate fetchers on the same protocol. On one hand browsers prevent file system access on the other hand node does not have native fetch. But both these things will go away. FS is coming to the browser and Deno has native fetch. My concern is that practicalities of today will lock in the design for many years with all that extra complexity.

Eventually we should strive to hide data storage itself from the user and for that Solid will have to be universal. And let me stress again that solid-rest is the best proof of this (and perhaps why I am eating your ear off!). Stated differently, solid-auth-client, solid-auth-cli or solid-auth-fetcher all should also just be clients of solid-rest since these are only just handlers.

Please just give it a little time, I think you will also come around to see beauty in the symmetry, I promise!!!!!!!

Networking example

For many applications ssh is an over-kill. Eg fetching some data from a NAS. Having said that I am going to reiterate that I lack clarity myself, so it is difficult to say much more for now!!!

jeff-zucker commented 3 years ago

I think this may solve your issue with being "at par" :

const client = new SolidNodeClient({                                            
   authHandler :    "",       // solid-auth-fetcher/solid-client-authn-node/...    
   protocolHandlers : {                                                         
     http : "",               // node-fetch/cross-fetch/...                        
     file :   "".             // solid-rest-file/...                               
     ...                                                                        
   }                                                                            
});                                                                             

Plain http is at par with the other protocols but the authHandler is privileged, not because it uses http, but because it provides login and authenticated services.

jeff-zucker commented 3 years ago

Or, to consider that there may be authHandlers for other protocols these would be the defaults :

new SolidNodeClient ({                                                          
  authHandlers : [                                                              
    { http : 'solid-auth-fetcher' },                                            
  ] ,                                                                            
  noAuthHandlers : [                                                            
    { http : 'node-fetch' },                                                    
    { file : 'solid-rest' },                                                    
  ], 
  parser : ""                                                                            
}) 
CxRes commented 3 years ago

As I mentioned above, a file system access requires authentication. As solid-rest matures, it is inconceivable that file system authentication will not be implemented despite the complex engineering needed to deal with the way file systems are designed at present. So you will end up with a situation that in your example will look like:

new SolidNodeClient ({                                                          
  authHandlers : [                                                              
    { http : 'solid-auth-fetcher' },
    { file: 'solid-rest-file-with-auth' }                                            
  ] ,                                                                            
  noAuthHandlers : [                                                            
    { http : 'node-fetch' },                                                    
    { file : 'solid-rest-file' }, // I think you meant solid-rest-file and not solid-rest here                                                    
  ], 
  parser : ""                                                                            
}) 

I would argue that you might as well get rid of this authenticated and non-authenticated handler distinction altogether. All authenticated handlers can access data from sources that do not need authentication. So we just end up with:

new SolidNodeClient ({                                                          
  handlers : [                                                              
    { http : 'solid-auth-fetcher' },
    { file: 'solid-rest-file-with-auth' }                                            
  ] ,                                                                            
  parser : ""                                                                            
}) 

I can see that there shall be handlers where authentication is meaningless, such as, localstorage. But such a handler is a special case of a handler with authentication, where the authentication is always true.

Close your eyes and imagine a computer that starts only with a solid browser only. What do you think is the best way to manage user identification across all resources, local or remote? How do we minimize user and programmer burden in using solid? You may have different answers, most probably better answers, but we need to have this conversation before too much gets baked in!

jeff-zucker commented 3 years ago

Well that's all true but leaves out the case where we do unauthenticated http fetches - which should happen automatically if the user is not logged in but which are replaced with authenticated fetches when the user is logged in. So we need to be able to specify both. How does this look? solid-node-client

jeff-zucker commented 3 years ago

There could be authenticated fetches for file:// or other protocols, but presumably those would also do unathenticated fetches so there is no reason to include two paths.

CxRes commented 3 years ago

I really like the chart, especially the bit where users can actually switch out handlers for the same protocol, which could be used to support different environments or performance characteristics. Just some confusion regarding http (that needs some clarification from you, see below).

Well that's all true but leaves out the case where we do unauthenticated http fetches - which should happen automatically if the user is not logged in but which are replaced with authenticated fetches when the user is logged in.

I did not follow. Could you please elabourate! Also I am confused by the second comment...

I would presume that you can always "fetch", whether you are logged in or not. The difference would be that you either send credentials in your request headers or don't. If you did not log in and placed a fetch, a response will still come, it just may not contain the data you are after. If you logged in later then you will just have to place a new fetch.

Once solid-auth-client has obtained its cookie (or another auth strategy), it also uses a fetch library underneath. Now that I think of it, it may be wise to just separate all auth logic from the fetch logic (though auth will itself use fetch to get identity from a server and feed that to subsequent fetches). Similar thing in terms of separation, though not the same, might be valid for the file-system as well (login the user using one code ie auth and then read data using another ie fetch). So this different model would look like:

new SolidNodeClient ({                                                          
  authHandlers : [                                                              
    { http : 'solid-auth-getter' },
    { file: 'solid-rest-system-auth' }                                            
  ] ,                                                                            
  fetchHandlers : [                                                            
    { http : 'solid-auth-fetcher' },                                                    
    { file : 'solid-rest-file' },                             
  ], 
  parser : ""                                                                            
}) 

But, I need to know first whats on you mind with your last two comments....

jeff-zucker commented 3 years ago

Yes, I think I am over-thinking it. I will remove the authenticated/unauthenticated distinction. Each handler for a protocol could do both auth/noAuth or only noAuth at its discretion.

jeff-zucker commented 3 years ago

solid-node-client2

CxRes commented 3 years ago

Looks good!

I'll try to submit a PR sometime this week wrt automatic initialization (though not necessary, i think should help the unsuspecting user).

Perhaps, you may consider splitting solid-rest to separate plugin repos at some stage!

jeff-zucker commented 3 years ago

Thanks for helping me think this through, I like the end result a lot.

jeff-zucker commented 3 years ago

Okay, so I've taken a stab at a plugin system, seems to work but maybe not the right way. Please take a look. See the README on plugins. https://github.com/solid/solid-node-client/tree/solid-client-authn

CxRes commented 3 years ago

You would need to explain some things:

Also, I think fallback should not be "node-fetch" but a piece of code that simply throws "no handlers associated with protocol foo:", like with the empty string case. Perhaps node-fetch might do that anyway as well as cover http: but, its confusing in that case and should be explained in documentation.

jeff-zucker commented 3 years ago

ESS is Inrupt's Enterprise Server. CSS is Community Solid Server - Ruben Verborgh's project that is not as feature rich but more spec compliant than NSS and is planned to replace NSS.

There is separate cookie fetching code because neither ESS nor CSS have or plan to have a command-line based method to obtain the token/secret. Getting the token/secret via a browser is the only option. I have spoken to Inrupt on this and they will not budge. So all I am doing is pointing people to Inrupt's bootstrap script (I also submitted a PR to Inrupt to improve that script).

I hear you on the fallback, I still think it is an appropriate name because someone may want to provide their own fallback library. There's no reason for us to check if a protocol exists, node-fetch will error on its own. For sure the docs could use some work. I've revised the README this morning plus mentioned you in the acknowledgements. Thanks again!

jeff-zucker commented 3 years ago

It is a fallback for both http: and unauthenticated https:. Among other reasons, it lets us not load the authenticated handler unless we need it but still work for https: requests.

CxRes commented 3 years ago

Getting the token/secret via a browser is the only option.

Isn't it still ultimately a https: request? At worst we could import the Inrupt cookie fetcher in https: handler internally and hide that from the user, maybe even persist it on disk.

Getting the token/secret via a browser is the only option. I have spoken to Inrupt on this and they will not budge.

Do you know whats going on here? Is it that they have a different idea of Solid than we do? Or is it some security thing? How does it matter what environment fetches Solid data?

The thing I hate about solid-auth-client is that actually does a redirect when accessing private parts of the pod, and opens the login window with a webpage. Ux should always be independent of the back-end negotiation and at discretion of the user/developer.

I still think it is an appropriate name because someone may want to provide their own fallback library.

It is a fallback for both http: and unauthenticated https:.

jeff-zucker commented 3 years ago

The token/secret is a security issue. It works this way in almost anyplace that requires you to get an store a secret or api-key.

They just released it, I am certainly not going to start building wrappers around it before they get further along.

Do not use solid-auth-client it is deprecated. Use Inrupt's solid-client-authn-browser instead. It does not support any kind of redirects but again, it is a security issue, and wiser people than me have tried to talk them out of it.

The current setup includes the possibility of defining an http handler. If someone wants that , they can just create it. That's a separate question of having a fallback. It's simply that, using the current defaults nothing handles http: so it goes to fallback.

jeff-zucker commented 3 years ago

If I have conditional requires, as I do in src/getAuthSession.ts, and I include those packages as devDependencies, not Dependencies, then downstream apps could import solid-node-client without those if it wanted (e.g. couldn't handle 'fs') but someone who npm installs the package gets all of the plugins installed and can use them out of the box. Maybe?

jeff-zucker commented 3 years ago

Also mark them as PeerDependencies.

CxRes commented 3 years ago

I have concerns with how you are coding:

jeff-zucker commented 3 years ago

solid-node-client will be a peer dependency of solid-rest-file,

I do not understand that at all. Solid-Node-Client instantiates a series of fetchers. Solid-Rest-File is one of those fetchers. SNC should import Solid-Rest-File, not the other way around. How would that work?

import() can be used in place of require()

AFAIK, not if the require should be conditional. Is conditional import possible? If not, how do we write Solid-Node-Client?

CxRes commented 3 years ago

Peer deps essentially say that if you want to use me, you need to fetch my host package. Read here: https://nodejs.org/en/blog/npm/peer-dependencies/

So if someone installs SRF, they need to get SNC (or a warning to get SNC). SNC on the other hand defines SRF as optional dep (because one can use SNC without SRF, if they know what they are doing).

Conditional Imports is very much possible and one of the use cases for Import() See: https://v8.dev/features/dynamic-import or other resources on the subject.

jeff-zucker commented 3 years ago

So if someone installs SRF, they need to get SNC (or a warning to get SNC). SNC on the other hand defines SRF as optional dep (because one can use SNC without SRF, if they know what they are doing).

Solid-Rest-File is a plugin for Solid-Rest. It is not meant to do anything at all on its own. There is no circumstance in which someone would want to install it on its own. It has no public user interface, only an interface that is called by Solid-Rest.

I'll read the docs on import, thanks.

CxRes commented 3 years ago

Can you clarify what is the final relationship between solid-rest and solid-node-client. I thought solid-node-client will call solid-rest with node-specific plugins and solid-rest will be requiring fully manual initialization?

jeff-zucker commented 3 years ago

Solid-Rest, by default, will load no plugins except those passed by the user or app that calls new SolidRest( [plugins] ). So if someone does not want fs and the other dependencies of solid-rest-file, they can load SolidRest without solid-rest-file.

Solid-Node-Client will, by default, instantiate a new Solid-Rest object with the Solid-Rest-File plugin and use that to handle file: requests.

Does that answer your question? Do you have a different way it should work?

CxRes commented 3 years ago

Ok! So you can then merely substitute solid-node-client with solid-rest wrt what I said about peer deps! Solid-rest-file etc. will be proper deps of solid-node-client. Does that sound right now?

jeff-zucker commented 3 years ago

Solid-Rest and Solid-Rest-File would both be dependencies of Solid-Node-Client. Solid-Rest-File would be an optional plugin for Solid-Rest, and not a dependency of it.

Does that sound right?

CxRes commented 3 years ago

Yes. And solid-rest is a peer dep of solid-rest-file!

CxRes commented 3 years ago

Also, I am closing this with #61.

jeff-zucker commented 3 years ago

Well, #61 is not going to be implemented. I will first re-write solid-node-client using dynamic imports. Then I will re-write solid-rest and make the plugins independent packages and remove the loading of defatult plugins and the environment checks.

jeff-zucker commented 3 years ago

Here is the problem with dynamic import : it requires async, so can't be called from constructor so new SolidNodeClient( some_plugin ) will not be able to use it. I guess I could make a installPlugin() which users would have to call after the call to new.

jeff-zucker commented 3 years ago

Or maybe this would work : only load the auth package on login - the package isn't needed before login, so there is no reason to load it before.

CxRes commented 3 years ago

Its ok, just that fix is available, even if this code goes into a maintenance branch subsequent a rewrite. You can accept it there or not, for history sake its there!

CxRes commented 3 years ago

Packages may be needed before login, but handler initialization/client creation can always be async! Eg node-fetch for http will not have a login.

jeff-zucker commented 3 years ago

How can it be async in a constructor?

jeff-zucker commented 3 years ago

Yes, right, node-fetch and solid-rest would be loaded automatically, only the auth packages would be delayed until login.

CxRes commented 3 years ago

You can do (internally or externally):

async function addHandler({protocol:handler})

user can or you can internally

client.addHandler(...)
jeff-zucker commented 3 years ago

Those can be done after the constructor but would not be backward compatible because users expect to be able to do login without calling addHandler() or whatever.

CxRes commented 3 years ago

I would import all the code at handler initialization and not wait till the user calls login.