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

Improve size of dependencies #47

Closed
timfish opened this issue Mar 18, 2018 · 4 comments
Closed

Improve size of dependencies #47

timfish opened this issue Mar 18, 2018 · 4 comments

Comments

@timfish
Copy link
Collaborator

timfish commented Mar 18, 2018

Most consumers of this library are not going to bundle it and its dependencies as they will access it directly from node_modules.

Currently it adds around 8MB to the app size and over 1MB to the compressed installer sizes of an empty Electron app.

It looks like we could save around 75% of this if we didn't have to include the whole of raven-js/dist.

image

@jan-auer
Copy link
Member

Looping in @kamilogorek @HazAT. I was wondering if we should just bundle raven-js and raven manually instead of adding it as a node module. Still haven't looked at how Electron bundles files inside node_modules, but maybe we can just exclude those files somehow...

@timfish
Copy link
Collaborator Author

timfish commented Mar 20, 2018

As far as I understand it, most electron packagers by default simply to prune --production the dependencies before packaging and include the entire node_modules directory after that.

electron-builder allows us to apply glob expressions during packaging. Some of these are quite aggressive and would likely break things for others:

# exclude all those build artifacts while ensuring we have the native binary
- "!**/node_modules/*/build/*"
- "**/node_modules/*/build/**/*.node"
# exclude some other native things
- "!**/node_modules/**/{*.cc,*.c,*.h,*.obj,*.pdb}"

I haven't checked but assuming @sentry/browser is using raven-js/dist/raven.js I could add the following to exclude everything else in dist:

- "!**/node_modules/raven-js/dist/*"
- "**/node_modules/raven-js/dist/raven.js"

But if the output of raven-js changes it breaks the above.

@jan-auer
Copy link
Member

I think it makes more sense to just check in a build of raven.js manually for now, until it is fully implemented in @sentry/browser.

@timfish
Copy link
Collaborator Author

timfish commented Apr 10, 2018

Much improved by v0.5.0 release

@timfish timfish closed this as completed Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants