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

@ntucker ntucker Jul 27, 2019

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.

Loading

// 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

@ntucker ntucker Jul 27, 2019

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.

Loading

@ntucker ntucker requested review from mprobber, nickcherry and fat Jul 27, 2019
@ntucker ntucker mentioned this pull request Jul 28, 2019
@ntucker
Copy link
Collaborator Author

@ntucker ntucker commented Jul 28, 2019

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

Loading

@ntucker ntucker force-pushed the optional-superagent branch from b455c9e to 8a62d2b Jul 28, 2019
@ntucker ntucker merged commit 192b7ea into master Jul 28, 2019
3 checks passed
Loading
@ntucker ntucker deleted the optional-superagent branch Jul 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants