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

[feature] Enable customizing networking library reducing bundle size #113

Merged
merged 2 commits into from
Jul 28, 2019

Conversation

ntucker
Copy link
Collaborator

@ntucker ntucker commented Jul 27, 2019

Fixes #103 .

Motivation

The core of rest-hooks clocks in at around 7kb gzip. However, there is one part - superagent - which is quite sizable. If someone decides to override their networking library it won't get tree-shaked out due to it being included in the Resource class. This results in what is essentially a doubling of this library's impact on their bundle size.

Solution

Splitting out the fetch implementation to a derived class allows those implementing their own fetch solution to not have to import the superagent fetch solution. This means with tree-shaking superagent will never even be included in the bundle.

Will have a follow blog post to talk about bundle sizes to clear things up since bundlephobia is pretty misleading and we have no desire to make the rest-hooks library worse just to rig artificial metrics that have no bearing in real life.

@@ -0,0 +1,366 @@
import { memoize } from 'lodash';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is actually just copy/paste of Resource but with rename of class.

// we type guarded abstract case above, so ok to force typescript to allow constructor call
const instance = new (this as any)(props) as AbstractInstanceType<T>;

if (instance.pk === undefined)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except this part was changed. This is better detection because there can be multiple abstract classes in the hierarchy beyond this.

@ntucker
Copy link
Collaborator Author

ntucker commented Jul 28, 2019

I'm very open to naming suggestions for the base class. (Known now as SimpleResource)

@ntucker ntucker merged commit 192b7ea into master Jul 28, 2019
@ntucker ntucker deleted the optional-superagent branch July 28, 2019 02:09
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

Successfully merging this pull request may close these issues.

Bundle size incorrect
2 participants