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

Switch to webpack #68

Merged
merged 45 commits into from
Jan 11, 2023
Merged

Switch to webpack #68

merged 45 commits into from
Jan 11, 2023

Conversation

Duddino
Copy link
Member

@Duddino Duddino commented Dec 30, 2022

Abstract

Moves to webpack. This deletes all of the external libraries to the repo (this makes it way easier to audit the code as well as updating libraries).
Convertes all of the existing code to ES6 modules. exported things in index.js will be available under the MPW namespace. This is only to access them in HTML, the namespace shouldn't be used in regular javascript.

To install dependencies:
npm i. To run webpack: npm run build. Then, the dist folder should be hosted/opened locally.
In a future PR the global.js file should be split in smaller files.

What features or improvements were added?

  • Removed a couple of redundant libraries
  • Translated the entire codebase to ES6 modules
  • Integrated webpack to the codebase

How does this benefit users?

  • Performance improvements by reducing code size by dropping a couple of redundant libraries and using the webpack in-built minifier.
  • Easier security audits by splitting the code into modules and dropping external minified libraries.
  • Improved security by making it easier to update libraries as well as implement unit tests.

@JSKitty
Copy link
Member

JSKitty commented Dec 30, 2022

Awesome! I'm sure this is gonna be a hell of conflicts and maintenance until merged, but this is absolutely the way MPW should evolve towards. 👍

@Duddino Duddino marked this pull request as ready for review December 31, 2022 18:08
Copy link

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK

Depends installs fine, the build is running fine.
Tested basic transactions sending/stake/unstake

Style nits on spacing as we're getting ready to have linting run by GitHub etc but otherwise great job!
I did not comment in regard to most spacing just because of seeing what others say

Copy link

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

And lastly the webpack.config.js is being ready as mixed with tabs and spaces

panleone
panleone previously approved these changes Jan 3, 2023
Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

tACK: tested with basically every single existing button of MPW (except for ledger) , awesome PR this will finally let us to start with shield transactions.

Copy link
Member

@JSKitty JSKitty left a comment

Choose a reason for hiding this comment

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

🎉 soft utACK, I've done a brief review of the entire set of changes, all seems fine and dandy, I highly appreciate the new file structure, lint style and module system, I will do actual code testing once the PR is 100% complete (i.e: once this includes the UTXO API merge, and no conflicts with master).

Amazing work. 💪 💜

Copy link
Member

@JSKitty JSKitty left a comment

Choose a reason for hiding this comment

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

tACK! 🎉

✔️ - Wallet Creation, Export and Import (Seed and Legacy).
✔️ - Send, Receive, Delegate, Undelegate.
✔️ - Cold Rewards recognised (Mainnet Tested Only).
✔️ - Masternode creation & start.
✔️ - All Settings.
✔️ - Mainnet and Testnet.

@JSKitty JSKitty requested review from panleone and Liquid369 January 11, 2023 19:10
@Liquid369
Copy link

tACK.
Screenshot 2023-01-11 at 1 57 29 PM
Screenshot 2023-01-11 at 1 57 47 PM

Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

tACK: tested again and it works fine, really good work :)

@JSKitty JSKitty merged commit 75b4229 into PIVX-Labs:master Jan 11, 2023
@JSKitty JSKitty mentioned this pull request Feb 3, 2023
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
Development

Successfully merging this pull request may close these issues.

4 participants