This repository has been archived by the owner on Jul 9, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 465
Reduce bundle-size of contracts-* packages #2330
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…idity source and lib dir (minus tests)
…artifacts/ts and wrappers.js
… all imports from src in contracts packages
|
fabioberger
force-pushed
the
refactor/reduceContractPkgBundle
branch
from
November 12, 2019 10:14
dd1cae3
to
30d5440
Compare
fabioberger
force-pushed
the
refactor/reduceContractPkgBundle
branch
from
November 12, 2019 10:52
628c104
to
efe8225
Compare
fabioberger
requested review from
abandeali1,
dekz,
feuGeneA and
hysz
as code owners
November 12, 2019 12:00
dorothy-zbornak
approved these changes
Nov 12, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, thanks! 💯
We should definitely do more cleanup on our end, like moving balance stores out of exchange
and into test-utils
, etc.
8 tasks
Merged
4 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR addresses the issue of the
@0x/contracts-*
packages bundle sizes with several refactors:.npmignore
in each package that excludes everything from publish except thelib/src
dir (until now we've been publishing everything: https://unpkg.com/browse/@0x/[email protected]/).test
dir is not published. Because of this, any exports from this dir that we still want exported were moved to thesrc
dir.index.ts
files. We want our public interface to be as small and concise as possible. These were replaced with specific imports. Protocologists, please develop a sensitivity to bundle size and what constitutes the public interface of a package 🙏 .artifacts
andwrappers
for every mixin, interface and test contract,@0x/contracts-gen
now generates all theartifacts
andwrappers
into thetest
dir of each package. Eachpackage.json
now contains apublicInterfaceContracts
config where we then pick which contracts we want to expose via the public interface. These contract artifacts/wrappers then get copied from thetest
dir to thesrc
dir as part of the build process.This refactor was made more difficult due to the prolific use of
import * from '../src'
(imports from index.ts) within packages. Please stop using this convention as it makes future refactors harder. It doesn't take that much work to explicitly import like one does with external imports.Results
@0x/contracts-exchange
Before:
After:
@0x/contracts-asset-proxy
Before:
After:
All packages experienced close to an order of magnitude size reduction.
Future improvements
There are still a handful of testing utilities that are exported from
contracts-*
packages. We should try and refactor the packages such that this no longer is the case. This should be done in a follow-up PR.Testing instructions
Types of changes
Checklist:
[WIP]
if necessary.