Avoid dependency on node-web-streams

[michielbdejong]

See https://github.com/comunica/comunica/issues/275#issuecomment-559049526

[michielbdejong]

The root problem is https://github.com/gwicke/node-web-streams/issues/1. That bug is almost old enough to go to school now ;) and unfortunately it's a dependency of Comunica, which is a dependency of LDFlex, which is a dependency of the generator, meaning that the generator cannot be installed in development environments that disallow installing dependencies from github links.

The maintainer of node-web-streams seems unresponsive, so maybe this can be fixed at the Comunica level? (see issue linked ^)

[RubenVerborgh]

Yes, we should definitely get it out. I have recently pruned some dependencies already of packages that were either unneeded, or had unnecessarily large webpackaged versions:

• https://github.com/solid/query-ldflex/commit/35cd221a99fad573e4a36791a85fd70bcf698166
• https://github.com/solid/query-ldflex/commit/82fc3113a71ced7a0d35bf2b14ef90c68c7b4162 (minus one mistake)

However, note that these are used with the webpack build process of LDflex to arrive at the browser build solid-query-ldflex.bundle.js. So libraries that use LDflex through imports, would have to incorporate a similar webpack configuration themselves. (For that purpose, I have exported the browser folder with the shims, as well as the webpack folder with configurations that can be reused partially as needed.)

As you can see, I've only done the trivial work so far. node-web-streams is a transitive dependency, so I did not look into it for now. However, given that there is no need for shims anymore in major browsers (https://caniuse.com/#feat=streams), I suspect we can just webpack it out as well.

[RubenVerborgh]

There is no problem with node-web-streams itself, it seems. It's a very small library with minimal impact. The problem is its dependency on web-streams-polyfill, which is no longer needed in modern browsers. As such, as can just webpack it away. I've done this for LDflex now: https://github.com/solid/query-ldflex/commit/4867ec17bd375e9380b8403f63d29f3e1e7849d4. This reduces the LDflex bundle from 944Kb to 896Kb—thanks for the suggestion!

It's now up to the generator project to take over those webpack configurations in its own builds.

[michielbdejong]

I think the steps to fix this would be:

1. Tag a version of comunica
2. Use that in LDFlex and bump the version there
3. Use that in Query-LDFlex and bump the version there
4. Use that in Solid React Components and bump the version there (also update the direct dependency on LDFlex there)
5. Use that here

[RubenVerborgh]

@michielbdejong Semver should normally just do its job, so a package-lock update could do?

[michielbdejong]

Ah yes! Then it might also work without steps 2, 3 and 4, and then in step 5 only update package-lock

[james-martin-jd]

So, I re-generated the package-lock locally and our tests began failing:

 ● Test suite failed to run

    Cannot find module '@comunica/actor-rdf-join-nestedloop' from 'comunica-engine.js'

    However, Jest was able to find:
        './comunica-engine.js'

    You might want to include a file extension in your import, or update your 'moduleFileExtensions', which is currently ['web.js', 'js', 'web.ts', 'ts', 'web.tsx', 'tsx', 'json', 'web.jsx', 'jsx', 'node'].

    See https://jestjs.io/docs/en/configuration#modulefileextensions-array-string

This error and similar is repeated many times.

[RubenVerborgh]

@rubensworks?

[rubensworks]

@james-martin-jd Are you using the latest @comunica/actor-init-sparql version (1.10.0)? Because that one should not depend on @comunica/actor-rdf-join-nestedloop anymore. If I inspect the published comunica-engine.js engine for that version, it contains no calls to @comunica/actor-rdf-join-nestedloop.

[Vinnl]

@rubensworks @james-martin-jd I got the same yesterday! A newly created React app, then as soon as I added @solid/react and imported LoggedIn, I got

./node_modules/ldflex-comunica/lib/comunica-engine.js Module not found: Can't resolve '@comunica/actor-rdf-join-nestedloop' in '(...)/node_modules/ldflex-comunica/lib'

This was when trying to run the app.

I couldn't reproduce it in a CodeSandbox so I assumed the issue would be on my side. I couldn't see those calls either though.

[RubenVerborgh]

Are you using the latest @comunica/actor-init-sparql version (1.10.0)?

@rubensworks I don't think it's the one we're using, but rather https://github.com/LDflex/LDflex-Comunica/blob/master/config/config-default.json (and https://github.com/LDflex/LDflex-Comunica/blob/master/package.json). Do we need changes there? Need to think about how Comunica Component.js configs affect semver, especially when such extra deps pop up.

[james-martin-jd]

@rubensworks we don't directly use comunica at all, our entry points to comunica, I believe, are through ldflex and possibly @solid/react?

An npm ls on the package you asked about shows:

solid-app@0.1.0 /Users/james/Projects/Solid/generator-solid-react/generators/app/templates
└─┬ @solid/query-ldflex@2.8.0
  └─┬ ldflex-comunica@3.2.2
    └── @comunica/actor-init-sparql@1.10.0 

[RubenVerborgh]

Fixed the above problem in ldflex-comunica@3.2.3; root cause is https://github.com/comunica/comunica/issues/592. Reassigning to @james-martin-jd for package-lock.json update.

[james-martin-jd]

Package lock was updated and should be in the 0.7.0 release going out hopefully today

[james-martin-jd]

After release, I am no longer seeing node-web-streams in an npm ls of the generated app, so I'm closing this ticket