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

Make brotli optional #320

Merged
merged 10 commits into from
Jul 9, 2019
Merged

Make brotli optional #320

merged 10 commits into from
Jul 9, 2019

Conversation

siddharthkp
Copy link
Owner

@siddharthkp siddharthkp commented Jul 3, 2019

Motivation and Context

From the looks of it, iltorb's native bindings are large + don't play well on all platforms - and fails installs on windows and some CI environments.

  1. For folks not using brotli compression at all, this is unnecessary overhead.
  2. Native brotli compression support landed in node 10.16.0 (LTS), we don't need to include an external dependency for that
  3. For folks using brotli and node < 10.16.0, this PR is a breaking change and will ask them to install bundlesize-plugin-brotli to continue

Changes introduced in this PR:

  1. Remove brotli-size from dependencies

  2. On Node >= 10.6.0, use zlib.brotliCompressSync

  3. On Node < 10.6.0, ask to install bundlesize-plugin-brotli

  4. Introduces bundlesize-plugin-brotli

Fixes #202, #283 and #213

Prior work: #220 #299

@siddharthkp
Copy link
Owner Author

Open questions

  • Should we merge this now or wait for 1.0.0 because of the breaking change?

@Haroenv
Copy link
Collaborator

Haroenv commented Jul 3, 2019

I guess it depends how many things we want to change still in a v1, I think it's a breaking change as well

@siddharthkp
Copy link
Owner Author

I guess it depends how many things we want to change still in a v1, I think it's a breaking change as well

True, might make sense to keep them all in a branch and release together.

@anikethsaha
Copy link

bundlesize-plugin-brotli

@siddharthkp You are making bundlesize plugable? if so, its just AWESOME

Plugins are the kind of best suites for Scalability.

@siddharthkp siddharthkp changed the base branch from master to beta July 9, 2019 10:16
@siddharthkp siddharthkp merged commit 5ce8b93 into beta Jul 9, 2019
@siddharthkp
Copy link
Owner Author

Merged into beta channel

@thomaswelton
Copy link
Contributor

It also looks like brotli size has been updated to drop iltorb
erwinmombay/brotli-size@8ca8a27

Is there a roadmap for release of your beta?

@siddharthkp
Copy link
Owner Author

Hi!

You might want to use this version of bundlesize that solves this issue: https://github.com/siddharthkp/bundlesize2#migration-from-bundlesize0180

it's not feature complete, but you might not need the missing features at all.

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

Successfully merging this pull request may close these issues.

4 participants