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

Reduce dependency size #844

Closed
wants to merge 3 commits into from

Conversation

emschwartz
Copy link
Contributor

@emschwartz emschwartz commented Feb 6, 2018

ripple-lib is quite a large dependency right now, in part because it includes some unnecessary files like tests and minified versions of the library. There's probably more that could be done to reduce the size (for example, the other ripple modules like ripple-address-codec also include tests in the published npm package), but this should be a start.

Related to interledgerjs/moneyd#10

the minified files are 17 Mb, which makes installing ripple-lib slower than it should be. those that want to include the minified files in their website should be able to build them and copy the compiled files over
@intelliot
Copy link
Collaborator

I was just thinking that this needed to be improved. Thank you!

Specifically, I noticed that the files in build were being published to npm; since those are the browser version of ripple-lib, I don't think npm should have them. Also, these files are referenced in package.json under files:

  • "bin/*",
  • "build/*",
  • "test/*",
  • "Gulpfile.js"

Should we remove these lines?

@FredKSchott
Copy link
Contributor

FredKSchott commented Feb 6, 2018

Awesome idea!

Right now package.json "files" is overriding .npmignore, so changes to the ignore file don't affect anything atm. This file should be removed so that it's less easy to confuse which one npm is using. In either case, "bin/*", "test/*", and "Gulpfile.js" can all be removed for 1.7+ MB savings. Woo!

As far as build/: this is related to #840: If we still want to support the frontend use-case, we need to support installing for the frontend via npm, either by pointing our package.json "browser" property to our prebundled ripple.js/ripple-min.js or by replacing the ws client with the standard WebSocket client or a WS library with browser support.

I'm counting 5 different bundled builds in build/, do we really need each of those 5? Can we get down to just ripple.js and ripple-min.js?

@FredKSchott
Copy link
Contributor

FredKSchott commented Feb 6, 2018

Also, adding/removing package-lock.json doesn't affect publish size :( We've been using a yarn.lock so far and I've had bad experiences trying to maintain and keep both up to date with each other.

For @types files, you want to keep them as-is so that they are published with their respective modules - in dependencies or devDependencies. For good rationale, see: microsoft/types-publisher#81.

@intelliot
Copy link
Collaborator

Thanks for those insights @FredKSchott ! I agree - I think we should put package-lock.json in .gitignore since we're already using yarn.lock.

I'm in favor of just 2 bundled builds for the browser. I don't know what's different about the "debug" build.. will have a look at that later.

@intelliot
Copy link
Collaborator

intelliot commented Feb 13, 2018

It looks like the debug build keeps most of the code as-is, while the normal build puts each file into a single line that gets evaled. If we went down to just ripple.js and ripple-min.js, would ripple.js be the current "debug" build?

Using present-day naming conventions:
I don't see a use case for ripple-latest.js. If you're deploying to production, use ripple-latest-min.js. If you're in development, use ripple-<version>-debug.js. Am I missing something?

@emschwartz
Copy link
Contributor Author

What do you think about publishing those builds as a separate module? I don't think you'll ever want the packed and unpacked versions in the same project, and those files are pretty large because they also bundle all of the dependencies.

@intelliot
Copy link
Collaborator

@emschwartz That's a good point. Now that we specify the browser-compatible shim for ws in our package.json, users are also free to browserify ripple-lib themselves.

Do you know of some other JS library that does something similar (so that we can follow their convention)?

intelliot added a commit that referenced this pull request Feb 13, 2018
- `"bin/*"`, `"test/*"`, and `"Gulpfile.js"` can be removed
- See #844
@FredKSchott
Copy link
Contributor

+1 for publishing the bundled lib separately, I know babel does this to some degree of success: https://www.npmjs.com/package/@babel/standalone

It will probably take some work to set up for ripple-lib, babel uses lerna to manage multiple packages in one repo but that's probably overkill for what we need.

@FKSRipple
Copy link
Collaborator

This was fun to go back and re-read :)

I'm not sure if this specific PR is still actionable, safe to close or spin off into an "improve perf" issue? /cc @intelliot

@intelliot
Copy link
Collaborator

@FKSRipple Good question. I'll close this PR for now, replacing it with #1057 , which does nearly the same thing. Anyone should feel free to open a new issue or PR if there's more (or something different) that should be done.

@intelliot intelliot closed this Oct 24, 2019
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