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

Proposal for web3.js: break up connection.ts into single function files under a subdirectory #23332

Closed
marcnjaramillo opened this issue Feb 24, 2022 · 5 comments
Labels
web3.js Related to the JavaScript client

Comments

@marcnjaramillo
Copy link
Contributor

Problem

Currently, connection.ts holds over 4000 lines of code and is cumbersome to navigate. It's possible to find a function using command + F / ctrl + F, but there are a number of similarly named functions so you still have to do some manual filtering or enter the complete function name - not necessarily a big issue, but it could be more efficient.

Proposed Solution

Split up the file into single-function files inside a connections folder. This would make it easier to find functions and ultimately it would be easier to maintain in the long run, especially if the codebase grows.

@jacobcreech
Copy link
Contributor

Do you want to split up the Connection class into multiple RPC actions? Something like how the Token class was split up into actions in @solana/spl-token?

How would you manage backwards compatibility? Would this help with solana-labs/solana-web3.js#1122?

@marcnjaramillo
Copy link
Contributor Author

marcnjaramillo commented Feb 24, 2022

I'm not familiar with what the Token class looked like previously, but after looking at it I think this is close to what I'm thinking. Was Token previously structured similarly to Connection?

For backwards compatibility, I'm not sure. I'm still pretty new to contributing to open-source and I haven't worked on anything like this before. I don't know how complicated it would be to ensure that these changes don't cause apps that currently use web3.js to break.

Ideally, refactoring Connection would help with #20673. If my understanding of something being tree-shakeable is correct we would want to make sure only the functions that are actually needed would get imported. Currently, we have to import the entirety of Connection even though we might only need two or three functions. If we take a similar approach that was used for @solana/spl-token, am I correct to assume that would help bring use closer to making @solana/web3.js tree-shakeable?

I'm going to spend some time researching this more in-depth, and I'll update my response with more details so we can have a better discussion.

@stellaw1
Copy link
Contributor

stellaw1 commented Mar 7, 2022

Potential solution here could be to move the exported types out into a different file - this can help shrink the size of connection.ts without introducing any major changes.

Ultimately, it seems like having a connection folder that houses an index.ts file that exports the functions in the Connection class broken up into separate files is the best solution. Breaking up the functions in the Connection class into different files that can be exported separately would also help make web3 more tree shakable (#20673)

This would be a major change in web3 and would probably lead to a version bump. Versioning can help with managing backwards compatibility

@mschneider
Copy link
Contributor

Just from the point of view of someone who developed on solana for a while. Don’t worry about backwards compatibility. People will switch if it reduces build size by >100kb as mobile becomes more and more of a use case for crypto. It’s more important to have a good solution for projects going forward as early as possible. Biggest hurdle to adoption will be to make sure anchor uses the new api

@steveluscher steveluscher added the web3.js Related to the JavaScript client label Dec 2, 2022
@steveluscher
Copy link
Contributor

This will happen naturally as part of solana-labs/solana-web3.js#1111.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
web3.js Related to the JavaScript client
Projects
None yet
Development

No branches or pull requests

5 participants