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

web3.js: TypeError regression #16207

Closed
armaniferrante opened this issue Mar 30, 2021 · 10 comments · Fixed by #16219 or #16253
Closed

web3.js: TypeError regression #16207

armaniferrante opened this issue Mar 30, 2021 · 10 comments · Fixed by #16219 or #16253
Assignees

Comments

@armaniferrante
Copy link
Contributor

armaniferrante commented Mar 30, 2021

In order to get my build to work once more, I need to change this line https://github.com/solana-labs/solana/blob/master/web3.js/src/system-program.ts#L674 to be programId: params.programId.toBuffer(). Otherwise I get an error TypeError: params.programId.toBytes is not a function. This seems to have regressed in the last day or so.

To reproduce, one can run

  • git clone https://github.com/project-serum/anchor
  • cd anchor/examples/spl/token-proxy
  • anchor test

After installing the dependencies here https://github.com/project-serum/anchor/blob/master/.travis.yml#L15.

@t-nelson
Copy link
Contributor

Likely related to #16179. cc/ @jstarry

@jstarry
Copy link
Member

jstarry commented Mar 30, 2021

Thanks for the report @armaniferrante!

I believe this is a symptom rather than an issue in itself. I think the root issue is that TypeScript is not picking up type definitions from @solana/web3.js/lib/types/index.d.ts if there is another older transitive dependency on @solana/web3.js which has the old hand-written type definitions wrapped in declare module '@solana/web3.js.

It's unclear to me how TypeScript expects library maintainers to handle the transition from a non-TS library to TS one since they don't recommend adding module declarations if your source is written in TypeScript. I think a reasonable fix is to post-process the generated TypeScript type definitions to add back the module declaration for now, similar to what I had to do here for Flow types: https://github.com/solana-labs/solana/pull/16190/files#diff-f19c8c93d627f3d50bad9193e516cf8891677c9e7068a822f28125560adec32a

@jstarry
Copy link
Member

jstarry commented Mar 30, 2021

@armaniferrante I'm working on a fix for this now

@jstarry
Copy link
Member

jstarry commented Mar 30, 2021

@armaniferrante this should be fixed when the next release v1.2.3 is published

@armaniferrante
Copy link
Contributor Author

I'm still getting the error, which happens at runtime. Some extra details (not sure if they're relevant):

  • My test is javascript, not typescript.
  • The modification I referenced above is to the transpiled web3.js/lib/index.cjs.js (rather than the .ts source).
  • web3.js is being used internally within another package, and so web3.js is not imported directly.

@jstarry jstarry reopened this Mar 30, 2021
@jstarry
Copy link
Member

jstarry commented Mar 30, 2021

Ok, I'll take a look tomorrow, thanks for the extra context @armaniferrante!

@jstarry
Copy link
Member

jstarry commented Mar 31, 2021

The issue is that your code is passing a PublicKey from web3.js v1.1.0 (@project-serum/anchor) to methods in web3.js v1.2 which require a new method on the PublicKey class. If you were using TypeScript, you would get a compile error complaining about the incompatibility of the two different classes. Ideally the same version of web3.js is used throughout your project, is that an acceptable solution @armaniferrante?

@armaniferrante
Copy link
Contributor Author

Sounds acceptable to me! I'll give it a shot. Thanks a bunch for investigating @jstarry.

@armaniferrante
Copy link
Contributor Author

Works like a charm. Hopefully I'm not speaking too soon, but the fix was this line coral-xyz/anchor@6576e0d#diff-43fffbeff5b448207741b2bd0e9a4872347c17dcb5eb7cf655aedd1e519349a2R77.

@jstarry
Copy link
Member

jstarry commented Mar 31, 2021

@armaniferrante after more thinking, this change technically breaks the interface by requiring pubkey inputs to have this new toBytes method and so shouldn't have happened in a minor release. I'm going to revert that behavior change to avoid the need for wrapping the public key class like you had to do

@jstarry jstarry reopened this Mar 31, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants