Skip to content
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

Make querying work in non-browser environments #12

Closed
julianrojas87 opened this issue Mar 21, 2019 · 18 comments
Closed

Make querying work in non-browser environments #12

julianrojas87 opened this issue Mar 21, 2019 · 18 comments
Assignees
Labels
bug Something isn't working

Comments

@julianrojas87
Copy link

When running the example available in the README in a node.js script, when executing the following line:

console.log(`This person is ${await person.name}`);

This exception is thrown:

(node:5934) UnhandledPromiseRejectionWarning: ReferenceError: window is not defined
    at currentUrl (/home/julian/Desktop/test-ldflex/node_modules/solid-auth-client/lib/url-util.js:9:42)
    at toUrlString (/home/julian/Desktop/test-ldflex/node_modules/solid-auth-client/lib/url-util.js:32:23)
    at SolidAuthClient.fetch (/home/julian/Desktop/test-ldflex/node_modules/solid-auth-client/lib/solid-auth-client.js:41:51)
    at ActorHttpSolidAuthFetch.run (/home/julian/Desktop/test-ldflex/node_modules/@comunica/actor-http-solid-auth-fetch/lib/ActorHttpSolidAuthFetch.js:13:22)
    at ActorHttpSolidAuthFetch.runObservable (/home/julian/Desktop/test-ldflex/node_modules/@comunica/core/lib/Actor.js:53:29)
    at MediatorNumber.mediate (/home/julian/Desktop/test-ldflex/node_modules/@comunica/core/lib/Mediator.js:80:22)
@RubenVerborgh RubenVerborgh transferred this issue from LDflex/LDflex Mar 21, 2019
@RubenVerborgh RubenVerborgh changed the title LDflex only works in the browser out of the box Make querying work in non-browser environments Mar 21, 2019
@RubenVerborgh
Copy link
Member

RubenVerborgh commented Mar 21, 2019

This is a consequence of our usage of solid-auth-client, which is a browser library. We probably want to fall back to either regular fetch (unauthenticated), or a proper library (we currently have the hack provided by https://github.com/solid/solid-cli/ nicely wrapped by https://github.com/jeff-zucker/solid-auth-cli).

We could do something similar to linkeddata/rdflib.js#306.

@rubensworks
Copy link
Collaborator

Similar to isomorphic-fetch, something like a solid-auth-isomorphic wrapper over solid-auth-client and solid-auth-cli may be good solution as well.

@RubenVerborgh
Copy link
Member

@jeff-zucker What do you think about the above?

We might want to have the correct config, such that https://github.com/linkeddata/rdflib.js/pull/306/files#diff-11e9f7f953edc64ba14b0cc350ae7b9dR45 happens automatically in all derived libraries, i.e., we don't want any browser version to include solid-auth-cli (jeff-zucker/solid-auth-cli#1).

@jeff-zucker
Copy link

One reason to prefer solid-auth-cli over isomorphic-fetch is that in a nodejs context, isomorphic-fetch falls back to node-fetch which falls back to an http.get() which makes it fail on file:// and data:// URLs whereas solid-auth-cli now supports file:// via file-fetch and I'm working on supporting data://. It uses isomorphic-fetch for non-auth https:// fetching. I should add that I am fully aware that solid-auth-cli is a hack and that once a clear path to something better becomes available I will either work to fix it or turn the module over to someone more capable. We can think of solid-auth-cli as a placeholder that allows switching between browser and browserless rather than as the currently existing module. If this is something you (@RubenVerborgh) want to do, I am glad to cede the namespace to you at any time. If it's something you'd like me to work on, I would greatly appreciate some guidance on how you see a real library working and what steps would need to occur before that is possible.

@jeff-zucker
Copy link

The idea of a solid-auth-isomorphic wrapper around both solid-auth-cli and solid-auth-client also sounds good.

@RubenVerborgh
Copy link
Member

I should add that I am fully aware that solid-auth-cli is a hack

…and just to be clear here, the fact that it's a hack is all on me 🙂, because of the dependency of https://github.com/solid/solid-cli/. With a more stable base, solid-auth-cli will also be stable. It's the best thing we currently have.

(More precisely, we depend on the server implementing app auth.)

If this is something you (@RubenVerborgh) want to do, I am glad to cede the namespace to you at any time.

You're doing a great job, so please keep going!

@julianrojas87
Copy link
Author

I made it work in a Node.js environment by reverting the change made to config/config-dafault.json in this commit. This configures Comunica to use the default http actor files-cais:config/sets/http.json which runs without issues in a non-browser environment.

@jeff-zucker
Copy link

Great! I'm wondering if we should implement a solid-auth-isomorphic anyway because a) it supports login and post/put/patch in nodejs b) we could support file:// and data:// and possibly other things in a way that both solid-auth-client and solid-auth-cli could use them.

@RubenVerborgh
Copy link
Member

RubenVerborgh commented Mar 22, 2019

@jeff-zucker Definitely (because the drawback of the other solution is that it removes auth).

@jeff-zucker
Copy link

I'm thinking of something like this:

solid-auth-cli.fetch
  if data url -> solid.auth.cli.dataFetch
  if file url -> solid.auth.cli.fileFetch -> file-fetch
  if not logged	in
    in browser -> solid-auth.client.fetch -> isomorphic-fetch
    in	nodejs	 -> solid-auth-cli._fetch -> isomorphic-fetch
  if logged in
    in browser -> solid-auth.client.fetch
    in nodejs  -> solid-auth.cli._fetch

@RubenVerborgh
Copy link
Member

@jeff-zucker I think you want "if browser" on the top level of the decision tree.

@jeff-zucker
Copy link

Right, the logged-in-or-not decision would happen in whichever modules was called depending on browser state. What about file and data though - wouldn't those behave the same in either context? I am thinking about supporting such things as data:text/turtle, and file globs so rdflib fetcher.load( file://path/* ) would behave as it does on Solid and put all RDF in the folder into the store. But maybe you can just call those from solid-auth-client if you want them.

@jeff-zucker
Copy link

I was thinking that the solid-auth-isomorphic wedge could overload your fetch and call data or file or super to your fetch in other cases.

@RubenVerborgh
Copy link
Member

What about file and data though - wouldn't those behave the same in either context?

File doesn't really exist in browser (CORS will bite you).

The reason for having browser at the top, is that those are the two versions you can ship.

@jeff-zucker
Copy link

Uh , duh yeah, CORS, the bane of existence. OK, nevermind :-).

@jeff-zucker
Copy link

Is this how you see it working?

rdflib, ldflex, otherBrowserAgnosticApps
    in package.json & webpack.config
        main: depends on solid-auth-cli/src
        browser: depends on solid-auth-cli/browser, pass-thru to solid-auth-client
    in code
        require solid.auth.cli  (which will be whichever library was depended on)
BrowserApps
    import solid-auth-client
NodejsApps
    require solid-auth-cli

@RubenVerborgh
Copy link
Member

@jeff-zucker Pardon the delay.
Yes, that's exactly what I would love to see.

@RubenVerborgh
Copy link
Member

@RubenVerborgh RubenVerborgh self-assigned this Aug 18, 2019
@RubenVerborgh RubenVerborgh added the bug Something isn't working label Aug 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants