Using browserify requires ignoreMissing

quadstorejs / quadstore

A LevelDB-backed graph database for JS runtimes (Node.js, Deno, browsers, ...) supporting SPARQL queries and the RDF/JS interface.

https://github.com/quadstorejs/quadstore

MIT License

203 stars 14 forks source link

Using browserify requires ignoreMissing #117

Closed gsvarovsky closed 3 years ago

gsvarovsky commented 3 years ago

Using browserify (and not webpack) it is necessary to add the option to ignoreMissing required files, so that the requires prefixed with _webpack_ignored_ in the quadstore-comunica bundle are ignored. Browserify doesn't seem to have an option to ignore by prefix or regex.

The bundle size has increased after the upgrade from 6.0 by about 600K.

jacoscaz commented 3 years ago

I was not aware of the ignoreMissing option of Browserify but, given the name, I would have expected it to achieve a similar result as this Webpack configuration. The bundle size of quadstore 7.x is significantly lower than that of version 6.x. I see two issues here:

[ ] figure out what is causing the bundle size to increase
[ ] the _webpack_ignored_ prefix is misleading, it should really be _comunica_ignored_

@gsvarovsky are you by any chance still requiring() https://github.com/beautifulinteractions/node-quadstore-sparql in your code? That's not needed anymore and it's the only thing big enough that I can think of that could be a remnant of version 6.x.

jacoscaz commented 3 years ago

@gsvarovsky could you test this with quadstore@7.1.0-beta.0, which I've just released to NPM with the beta tag? In addition to embedding the typings for comunica, I've changed the way the bundling mechanism works so that it does not result in having to ignore packages anymore.

gsvarovsky commented 3 years ago

I am getting a compiler problem building my project:

m-ld-js % npx tsc
node_modules/quadstore-comunica/index.d.ts:8:22 - error TS2344: Type 'K' does not satisfy the constraint 'string | number | symbol'.
  Type 'K' is not assignable to type 'symbol'.

8   toObject(): Record<K, V>;
                       ~

gsvarovsky commented 3 years ago

Incidentally I realised that I am using npm link to bring the m-ld JS library project into the website project. This may explain the bundle size, as there are two copies of quadstore in npm_modules. I'll use a tar bundle temporarily to see if this explains it.

Hmm. It's a bit smaller now, still ~400K bigger than before

jacoscaz commented 3 years ago

@gsvarovsky apologies, for some reason that error did not show up in my testing. I've just published 7.1.0-beta.1 which should also fix another issue related to the bundle. Could you try that?

gsvarovsky commented 3 years ago

OK great news.

My bundle size change has nothing to do with quadstore. Using source-map-explorer I see: quadstore-comunica/index.bundle.js • 340.91 KB quadstore • 48.38 KB (via browserify, unminified) Many apologies! I must discover what else has changed in my world.
The ignoreMissing flag is no longer needed with quadstore@7.1.0-beta.1

Thanks, happy to close this ticket!

jacoscaz commented 3 years ago

Great! Issue closed. 7.1.0-beta.1 should also fix https://github.com/beautifulinteractions/node-quadstore/issues/116 by the way.