8000 feature: add an alias for the technology preview of the new web3.js by steveluscher · Pull Request #163 · solana-playground/solana-playground · GitHub
[go: up one dir, main page]
Skip to content

feature: add an alias for the technology preview of the new web3.js #163

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feature: add an alias for the technology preview of the new web3.js #163

wants to merge 4 commits into from

Conversation

steveluscher
Copy link
Contributor
@steveluscher steveluscher commented Nov 7, 2023 •
edited
Loading

Summary

By no means a complete solution, this PR is the start of adding support for the Technology Preview version of @solana/web3.js.

Here, we import the tp version of the library and alias it to web3tp.

More on the new web3.js, here: https://www.youtube.com/watch?v=JUJtAPhES5g

Test Plan

  1. Create a wallet and airdrop some SOL to it.

  2. Create a client like so:

    import "@solana/webcrypto-ed25519-polyfill";

// Client
const rpc = web3tp.createSolanaRpc({
  transport: web3tp.createDefaultRpcTransport({
    url: "https://api.devnet.solana.com",
  }),
});

const keyPair = await web3tp.generateKeyPair();
console.log(keyPair);

console.log("My address:", pg.wallet.publicKey.toString());
const { value: balance } = await rpc
  .getBalance(pg.wallet.publicKey.toBase58())
  .send();
console.log(`My balance: ${Number(balance) / web3.LAMPORTS_PER_SOL} SOL`);

  3. Observe output

    Running client...
  client.ts:
    { privateKey:
   { algorithm: { name: 'Ed25519' },
     extractable: false,
     type: 'private',
     usages: [ 'sign' ] },
  publicKey:
   { algorithm: { name: 'Ed25519' },
     extractable: true,
     type: 'public',
     usages: [ 'verify' ] } }
    My address: 9kShTNxtemuHJzsXBmSPpS4o8vSdPAgBf9uEdAadm9j6
    My balance: 0.01 SOL

image

@steveluscher steveluscher mentioned this pull request Nov 7, 2023
Merged
@acheroncrypto acheroncrypto added the client Client related issues label Nov 8, 2023
acheroncrypto
acheroncrypto reviewed Nov 8, 2023
View reviewed changes
Copy link
Member
@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Thank you for adding this.

More on the new web3.js, here: https://www.youtube.com/watch?v=JUJtAPhES5g

Great talk 👏

https://user-images.githubusercontent.com/13243/281195443-4d112114-ca79-49ae-aa4b-d014fbc90a3b.png

Judging by the image(web3tp.generateKeyPair()), do some types not work as expected?

@steveluscher
Copy link
Contributor Author

Judging by the image(web3tp.generateKeyPair()), do some types not work as expected?

This thing definitely does not work with TypeScript 4.x. I'm trying to upgrade TypeScript in the playground right now (and thereby vscode to the ts5.2 tag) but I don't really understand how this thing works.

@acheroncrypto
Copy link
Member
acheroncrypto commented Nov 8, 2023 •
edited
Loading

This thing definitely does not work with TypeScript 4.x. I'm trying to upgrade TypeScript in the playground right now (and thereby vscode to the ts5.2 tag) but I don't really understand how this thing works.

I think this particular issue is caused by types being exported from dependencies but currently, playground only imports the package itself and not its transitive dependencies. For example, if we were to declare the @solana/keys package as a global package, web3tp.generateKeyPair() would exist.

Declaring the packages in supported-packages.json is a massive abstraction to declare the types and most of it is handled by us. Declaration related code is in here but this probably won't be straight forward to update so it's probably best if I take care of the types later and save you some time.

There was an issue with the build when I last updated the monaco-editor(dd804c5) and typescript(78156b8) dependencies. I wasn't sure what caused the issue(likely craco) so I downgraded to the latest working version, maybe it works now?

@steveluscher
Copy link
Contributor Author

Wow, I'm really glad you know how all this fits together so that I don't have to go figure it all out :)

Let me know if there's anything more I can do on this PR!

@acheroncrypto
Copy link
Member

Thanks, the PR looks great but is the @solana/web3.js-tp the only package we want to expose?

I think we'd want the transitive types working before this PR otherwise it will be very difficult for people use it.

@steveluscher
Copy link
Contributor Author

Yeah, for sure difficult.

Sorry, I'm just dumb – I can't parse whether “it's probably best if I take care of the types later and save you some time” means:

  • Steve should fix the types.
  • Let's land this now and I'll fix the types later.
  • I'll fix the types, push them to this branch, and we'll land the whole enchilada at once.

@acheroncrypto
Copy link
Member

Sorry I wasn't clear, I meant that I'll fix the types(landing the whole enchilada at once option) but I can only start after I finish what I'm currently working on because I'm not sure how long it will take to fix the types.

Of course, you can also fix it if you're feeling extra adventurous.

@acheroncrypto acheroncrypto mentioned this pull request Nov 9, 2023
Merged
acheroncrypto added a commit that referenced this pull request Nov 10, 2023
@acheroncrypto
Add type support for transitive client dependencies
e41c1b2 
One step closer to the new `web3.js` #163
@acheroncrypto
Copy link
Member

I didn't want to make you wait so I added transitive deps in e41c1b2 (ended up being easier than I thought)

web3-tp-transitive

Pretty nice that playnet still works with the new lib(url: "http://playnet")

There are still some type issues but this solved the majority of them.

acheroncrypto added a commit that referenced this pull request Nov 12, 2023
@acheroncrypto
Fix importing types from packages with file paths
1eb7f23 
* `monaco-editor` had a bug(microsoft/monaco-editor#2295) where paths with `@` wouldn't work, this seems to have been fixed in microsoft/monaco-editor#3057

* Types from package name with file paths e.g. `import type { Rpc, RpcSubscriptions } from '@solana/rpc-transport/dist/types/json-rpc-types';` now works properly

* We can now get the types for the RPC methods in `@solana/web3.js-tp`(#163)
@acheroncrypto
Copy link
Member

Type imports from package names with file paths e.g. @solana/rpc-transport/dist/types/json-rpc-types now works(1eb7f23). RPC methods also work after this change:

web3-tp-rpc

Problems

  • Types for the RPC method parameters exist but the first overload for many of the types are typed as ...args: unknown[] and the remaining are correct. In the image above, all 24 overloads are correct for getBlock but not for getBalance
  • Types for SubtleCrypto such as CryptoKeyPair are missing
  • After typing rpc. there is ~1s delay before suggestions show up, I thought this could be happening only in playground so I tried the same code in VSCode locally and noticed a similar delay when dealing with the RPC methods

@steveluscher
Copy link
Contributor Author

After typing rpc. there is ~1s delay before suggestions show up…

1 second? That's pretty awesome. It used to be 14. In all seriousness, though, presumably it's slower here than with an LSP because typechecking is happening on the client side here, right? I'm hoping that we can hold the dam back until TypeScript resolves microsoft/TypeScript#14107

…but not for getBalance

Oh, weird. In my editor (TypeScript 5.2.2) it looks like this.

image

Is this a TypeScript version problem? It's 4.x that's running in playground, right? Seems like that interface has been in the DOM types for quite a while.

@acheroncrypto
Copy link
Member

1 second? That's pretty awesome. It used to be 14. In all seriousness, though, presumably it's slower here than with an LSP because typechecking is happening on the client side here, right?

14 -> 1 sounds great to hear 😅 and yes, it's happening on the client side and it's a bit slower because it's running in a browser but the type resolution is usually fast enough that you wouldn't notice the difference.

I'm hoping that we can hold the dam back until TypeScript resolves microsoft/TypeScript#14107

Looks like it's going to get resolved anytime now 👍

Oh, weird. In my editor (TypeScript 5.2.2) it looks like this.

Yeah, this seems to be a playground specific problem, haven't debugged yet.

Is this a TypeScript version problem? It's 4.x that's running in playground, right? Seems like that interface has been in the DOM types for quite a while.

DOM adds a lot of unnecessary types for playground usage so it's not enabled currently but we can easily add the types we need. Are there any other types we need from DOM besides the crypto types?

@steveluscher
Copy link
Contributor Author

Are there any other types we need from DOM besides the crypto types?

I removed "DOM" from all the tsconfigs and ran the typechecker to get an idea of what else. Got errors for AbortSignal, AbortController, CryptoKey, CryptoKeyPair, SubtleCrypto, DOMException, AbortSignalEventMap, EcKeyGenParams, RsaHashedKeyGenParams, CloseEvent, WebSocket, MessageEvent, KeyUsage, JsonWebKey, and BufferSource, but many of those probably don't make it to the external API so ¯\_(°ペ)_/¯

@acheroncrypto
Copy link
Member

I removed "DOM" from all the tsconfigs and ran the typechecker to get an idea of what else. Got errors for...

Thanks, I've added a lighter version of DOM types and it seems to work great now.

Only the incorrect rpc method types issue remains. It's interesting that getBlock works even though it seems to be the most complicated one with 24 overloads — any idea why that could the case? This works as expected:

getBlock

but other methods have (...args: unknown[]): PendingRpcRequest<unknown> type as an extra overload:

getBalance

overload types also work:

getBalance-correct-overload

but it's just that the first overload is incorrect which makes the return types unknown too.

I've tested it with different TypeScript and monaco-editor versions too but it made no difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acheroncrypto acheroncrypto Awaiting requested review from acheroncrypto

Assignees
No one assigned
Labels
client Client related issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@steveluscher @acheroncrypto
0