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

Using browserify requires ignoreMissing #117

Closed
gsvarovsky opened this issue Nov 19, 2020 · 7 comments
Closed

Using browserify requires ignoreMissing #117

gsvarovsky opened this issue Nov 19, 2020 · 7 comments

Comments

@gsvarovsky
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Owner

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

gsvarovsky commented Nov 20, 2020

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
Copy link
Owner

@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
Copy link
Contributor Author

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
Copy link
Owner

Great! Issue closed. 7.1.0-beta.1 should also fix #116 by the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants