solid-client-authn-js icon indicating copy to clipboard operation
solid-client-authn-js copied to clipboard

`cross-fetch` overrides `window.fetch` with non-standard behaviour

Open NSeydoux opened this issue 5 years ago • 3 comments

Bug description

When using solid-auth-fetcher in a modern browser, the used fetcher should be provided by the browser itself. However, it seems that the polyfill returned by cross-fetch does not default to window.fetch, and has non-standard behaviour.

What prompted reporting this bug specifically is the fact that the obtained Response doesn't have a .body method to access its content as a ReadableStream.

To Reproduce Steps to reproduce the behavior:

  1. If you don't have a React sandbox, create one (npx create-react-app my-app)
  2. Install cross-fetch
  3. Make sure useEffect is imported in App.js
  4. import the cross-fetch fetcher: import {fetch as crossFetch} from "cross-fetch"
  5. Add this code snippet to the App function:
useEffect(() => {
     const buildFetcher = async () => {
      let fetcher;
      if(typeof window !== undefined && typeof window.fetch !== undefined) {
        console.log("Using the window fetcher")
        fetcher = window.fetch;
      } else {
        console.log("Using cross-fetch")
        fetcher = crossFetch;
      }
      fetcher("https://ruben.verborgh.org/profile/")
      .then(response => response.body)
      .then(stream => stream.getReader())
      .then(reader => {
        return reader.read().then(function processText({ done, value }) {
          if (done) {
            console.log("Stream complete");
            return;
          }
          console.log(value);
          return reader.read().then(processText);
        });
      });
    }
    buildFetcher();
  });
  1. Run the app: npm run start

Expected behavior

  • The code prints "Using the window fetcher", and then the stream content (byte arrays). This is what happens with the provided code snippet.
  • When replacing fetcher("https://ruben.verborgh.org/profile/") with crossFetch("https://ruben.verborgh.org/profile/"), to force using the polyfill, then an error occurs, "TypeError: stream is undefined", because the response returned by cross-fetch does not implement the body method.

Desktop (please complete the following information):

  • OS: Ubuntu 18.04
  • Browser: Firefox 77.0.1, Chrome 83

Additional context

LDflex expects the response to be readable as a stream. Also, this might have been the root cause of #88, and all the discussion around non-standard headers.

NSeydoux avatar Jun 10 '20 09:06 NSeydoux

Can/should we just webpack cross-fetch out?

RubenVerborgh avatar Jun 10 '20 11:06 RubenVerborgh

got a conflict too with fetch as somewhere else in the same code page I use the basic browser fetch https://developer.mozilla.org/fr/docs/Web/API/Fetch_API/Using_Fetch to get wikidata entities

const wikidata = 'http://www.wikidata.org/entity/'
const API_URL = 'https://www.wikidata.org/w/api.php?action=wbgetentities&origin=*&format=json'
let language =  navigator.language.split("-")[0] || 'en'
language+='|en'
const splitext = text.split(wikidata)[1]
const search_url = API_URL+"&ids="+splitext+"&props=labels&languages="+language
const res = await fetch(search_url)
const json = await res.json()
let label
try{
   label = json.entities[splitext].labels[language] != undefined ? json.entities[splitext].labels[language].value : json.entities[splitext].labels.en.value
 }
  catch(e){
     console.log(e,json.entities)
  }
 return label

and the basic browser fetch was replaced with the one of the lib the work around i found is to import with import * as sc from '@inrupt/solid-client-authn-browser' https://github.com/scenaristeur/vatch-vue/blob/8d3900638671324295d83286c256d48344028a45/src/plugins/vue-solid.js#L36

then use const myDataset = await getSolidDataset( url, {fetch: sc.fetch}); but it implies to add 'sc.' to all other function or to make to imports, one for fetch and the other for other functions

but is it a good thing that this lib replace the default behaviour of the browser ?

scenaristeur avatar May 23 '21 22:05 scenaristeur

@scenaristeur If that works, possibly import { fetch } as scaFetch from "@inrupt/solid-client-authn-browser works too, to avoid having to prepend sc. everywhere?

It should indeed not be modifying the global fetch.

Vinnl avatar May 24 '21 09:05 Vinnl