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

Optimizing npm dependencies #607

Open
1 task
ChiaMineJP opened this issue Jan 4, 2022 · 0 comments
Open
1 task

Optimizing npm dependencies #607

ChiaMineJP opened this issue Jan 4, 2022 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@ChiaMineJP
Copy link
Contributor

ChiaMineJP commented Jan 4, 2022

There are several npm dependencies which are not used at all or should be replaced with simpler lines of code.
This issue is to manage the tasks required to optimize npm dependencies.

Removable npm dependencies without updating any code.

Just remove them by npm uninstall. There should be no side-effect.

  • @material-ui/styles
  • byte-size
  • npm-audit-resolver
  • polished
  • stream-browserify
  • stringify
  • tail
  • yarn

npm dependencies which should be replaced/rewritten by simpler lines of code

  • crypto-browserify
    Large part of this npm module is redundant. crypto-browserify is used to provide NodeJS’s crypto equivalent into browsers. Albeit this module provides a lot of crypto modules, chia-blockchain-gui is only using randomBytes function. A test file(tests/util/header.test.js) also calls require(‘crypto’), but even in that case only createHash is used. I believe we can remove crypto-browserify from dependencies list by replacing it with smaller npm packages which implement only required features for us.
  • match-sorter
    I believe Array.filter function can replace the whole package in case of chia-blockchain-gui.
  • moment
    This package brings huge amount of files on building final output. We should replace this by alternative light weight package or write our own code for date formatting.
  • react-use
    License: Public Domain. A collection of 95 React Hooks. chia-blockchain-gui uses only 6 out of 95. 5 hook functions are less than 20 lines and 1 hook function is just 65 lines. It is really easy and legal to copy the code into our own to reduce unnecessary dependencies. If another functionality in react-use is required you can copy the lines as well because its license is public domain. List of 6 hooks we’re using: useToggle, useCopyToClipboard, useEffectOnce, useInterval, useAsync, useUpdate
  • validator
    MIT license. In the current code, only isNumeric and isURL are used. These validations are simple and we can replace them with our own code. Other than above 2 validations, more than 600KB unused files will be downloaded when running npm install. By rewriting with our own code, we can save 600KB on setup and isolate vulnerabilities in unused lines.

Discussion required

The whole discussion can be summarized into one subject. "Should we support legacy browsers?". UI/UX part of chia-blockchain-gui is written in React and designed to be isolated from platform. But supporting legacy browsers like IE11 is too inclusive, in my point of view.

  • core-js
    Currently the GUI app runs only on Electron. core-js is usually used to absorb incompatibility between browsers but as of the case for the Chia GUI app, only Chromium browser on Electron is supported. Removing core-js may cut support for old browsers with the benefit of greatly reduced build file size. Should we really keep core-js on the dependency list for old browsers support?
  • isomorphic-fetch
    This package introduces browser's fetch functionality to NodeJS. It seems fetch in chia-blockchain-gui uses window.fetch provided by Chromium's window interface. Unless we have a plan to make chia-blockchain-gui run on old browsers like InternetExplorer, isomorphic-fetch should be removed.
  • regenerator-runtime
    This is used to polyfill for old browsers which do not support the async / await keyword and JavaScript generator function. In other words, this package is mainly for Internet Explorer. According to https://caniuse.com/es6-generators and https://caniuse.com/async-functions, 96.64% of all users use browsers which support the generator function, and 95.61% of all users use browsers which support the async function. I think we should remove this polyfill package. Maybe at some point @babel/polyfill was used in chia-blockchain-gui. But it was deprecated and replaced with corejs/stable and regenerator-runtime/runtime as https://babeljs.io/docs/en/babel-polyfill told to do so.

Further investigation required

  • react-number-format
    Over 1000 lines of code. I couldn’t decide easily whether to replace this module. Currently, this module is used to format a number of XCH(mojo). But I saw only the following features are used:
    • Thousand separator
    • Disallow negative number
      If only using such simple features, I think we should and are able to replace this module with a smaller custom implementation of our own.
  • react-router
    https://github.com/remix-run/react-router/tree/main/packages/react-router
    As the above statement says, everything we need for routing should be imported from react-router-dom. Enlisting react-router to direct dependency in package.json is only required when we extend core functionality of react-router.
    I think we can remove this package from package.json after I make sure whether such a core extension exists.
  • react-scroll-to-bottom
    At first I thought this was a simple module. But it turned out to be a complicated one and it nearly reached 1,000 lines. I didn’t expect it would require time to to understand the core behavior/usage of this package.
    I’ll investigate further after other investigations are completed.
@ChiaMineJP ChiaMineJP self-assigned this Jan 4, 2022
@seeden seeden added the enhancement New feature or request label May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants