-
Notifications
You must be signed in to change notification settings - Fork 141
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
Added Solid support #1319
base: master
Are you sure you want to change the base?
Added Solid support #1319
Conversation
Great to see a PR for another open protocol! Feel free to also notify people on the community forums about these PRs: https://community.remotestorage.io/t/adding-solid-as-a-backend/828 Maybe you could also add some information for people new to SOLID (like myself e.g.) about how to test this end-to-end (i.e. which server implementation and.or popular provider/hoster to use). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wonderful; thank you!
A big change like this will take us a while to review and may take a couple rounds of changes (hopefully small); please be patient.
1. Have you had a chance to run the API test suite against this? rs-backup tweaked for your server is the easiest way to generate a token, that I've found.
-
Do you know if the Solid library is using fetch or XHR? We've been supporting environments with either one; if Solid only uses fetch we'll need to document that.
-
I see you've written tests using Jaribu. Unfortunately, that has been deprecated and doesn't run under recent versions of Node.js. New tests are being written using Mocha. I know it's a pain, but could you please add Mocha tests for Solid?
src/interfaces/configObserver.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add some comment lines, explaining how this file fits into the architecture?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this interface is only used for Solid code, then you can move it to the Solid module and export it from there.
@@ -368,7 +370,7 @@ export class RemoteStorage { | |||
if (typeof cfg === 'object') { extend(config, cfg); } | |||
|
|||
this.addEvents([ | |||
'ready', 'authing', 'connecting', 'connected', 'disconnected', | |||
'ready', 'authing', 'connecting', 'pod-not-selected', 'connected', 'disconnected', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can imagine some reasons for 'pod-not-selected' to be separate from 'error', but please detail them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... because no existing app will respond to the 'pod-not-selected' event, but they will respond to other events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be the state of the Solid connection process for the widget or someone implementing custom UI. So it should be part of a documentation page for adding Solid to an app (maybe another section on the GDrive/Dropbox page, and renaming the page and menu item).
The API test suite is irrelevant here, as it tests the compliance of RS servers with the RS specification. However, this PR is for clients connecting to Solid servers, as an alternative to RS servers. |
Another note for tests: there is a new Mocha suite for the RemoteStorage class in this open PR: #1318 |
Right you are! I've been wrestling with Armadietto for half a year. :-S |
Maybe I need to add a full documentation on how to directly incorporate the Solid backend without using the widget?
It does. The object that has the method is
I'll fix it. I believe I renamed the file to
const connectTask = setTimeout(() => {
remoteStorage.solid.setAuthURL('your-solid-identity-provider'); // i.e. https://login.inrupt.com
remoteStorage.solid.connect();
// Calling 'connect()' will immediately redirect to your identity provider website.
}, 1000);
remoteStorage.on('pod-not-selected', () => {
clearTimeout(connectTask);
const podURLs = remoteStorage.solid.getPodURLs();
// Choose one. Maybe there is even 0?
remoteStorage.solid.setPodURL(podURLs[0]);
// That's it. 'connected' is fired immediately.
};
remoteStorage.on('connected', () => {
// We are connected.
clearTimeout(connectTask);
// We arrive here either through calling 'setPodURL' on the `pod-not-selected` event or
// on page getting loaded. Because: Inrupt Solid library prefers to always redirect to the
// identity provider to get the session tokens. I spent a lot of time on this. If we want to
// change it, we'll have to make fundamental changes to Inrupt's library and make a PR.
}); Calling |
Great! Please add a comment in the code and update the event documentation, to that effect. |
If you have an app that uses this, it would help if we could look at it. |
I believe the vast majority of browsers in use support fetch, but the API didn't fully stabilize in Node.js until v21. Security support for v20 ends on 30 Apr 2026 (in 1 year and 8 months) If I understand correctly, the implementation in v18 and v20 doesn't support streams fully. So, we'll need to fully test this under Node 18 and 20, to see if this works properly with them. If it doesn't, I'm okay with documenting that this version of remotestorage.js when used in Node.js, should only be used with v21 and later. |
According to our docs, |
I've updated the documentation and added mocha tests. This should be it as far as I followed the conversation. What do you guys think? |
I haven't been able to use this in a real app yet — the app I've been testing with is throwing up issues that don't appear to be caused by this. I'll have to try with a different app. |
Hi there, I don't know much about remoteStorage; but I've been working on Solid Apps and something crucial to Solid is using RDF for interoperability. Seeing this PR, I understand that this adds support for a Solid backend as a "file storage", right? I wonder how would anyone use this library to make a Solid application using RDF. Would the developers need to write RDF manually? What about updates to documents? Anyways, I'm just mentioning this in case you weren't aware. If that's already the intention, using Solid as a file storage rather than a store of RDF documents, that's fine. But I think using a Solid POD like that is missing a lot of the potential. I guess it's better than nothing though, if it adds "Solid support" to some of the existing apps implemented using remoteStorage. But I don't think I'd recommend anyone making a Solid App from scratch to follow this approach. By the way, in case it's useful for anyone, some years ago we had a discussion comparing remoteStorage, Fission, and Solid. And this was one of the points that came up: https://vimeo.com/779640162 |
Yes, you won't get many of the benefits of a native Solid app. But this is an important step in moving people to storage independent of apps. With this, someone who has a Solid pod can also use the same storage for Apps written for remoteStorage. |
Wouldn't that be something that could be supported in the data modules for this library? |
This was the idea when we were considering the task. The data module receives the Client instance which probably exposes the RemoteStorage hence the Solid backend which then you can get the raw Solid Session from it. So a data module can implement a specific behavior for dealing with Solid the way it is recommended. The downside is that in order to be Solid friendly is that the different data modules need to have a check for the back-end type and do different thins. |
I've tried adding this and the updated widget to a simpler app, but no option for Solid storage appears in the list. Is there an example of this running in a real app? |
I'm trying this out in remotestorage/myfavoritedrinks#15 |
There seems to be a bug related to folder listing. Storing 'beer' as my favorite drink works, but then it repeats
|
I found a bug in this PR. The code that produces folder item maps is not including ETags (lines 414 and 419 of src/solid.ts) |
Hello, I have implemented the Solid backend. These changes are compatible with the widget. I am creating another pull request for the widget as well.
The test code and documentation updates are also there...
Thank you :-)