LDflex / LDflex-Comunica

Comunica query engine support for the LDflex language
MIT License
16 stars 5 forks source link

Make querying work in non-browser environments #12

Closed julianrojas87 closed 5 years ago

julianrojas87 commented 5 years ago

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 commented 5 years ago

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 https://github.com/linkeddata/rdflib.js/pull/306.

rubensworks commented 5 years ago

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 commented 5 years ago

@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 (https://github.com/jeff-zucker/solid-auth-cli/issues/1).

jeff-zucker commented 5 years ago

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 commented 5 years ago

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

RubenVerborgh commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

jeff-zucker commented 5 years ago

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 commented 5 years ago

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

jeff-zucker commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

jeff-zucker commented 5 years ago

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 commented 5 years ago

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

RubenVerborgh commented 5 years ago

Fixed in https://github.com/solid/solid-auth-client/commit/008518ecd14dd4273ef6127ad6042b99cdcab0ef