Skip to content

Conversation

@chadoh
Copy link
Contributor

@chadoh chadoh commented May 14, 2024

What

Uses the in-progress branch at stellar/js-stellar-sdk#962, which will be merged and released as 12rc3 soon. This PR can be updated to use the release at that point.

Uses the latest stellar-sdk@12.0.0-rc.3

Why

Fixes #1285

Known limitations

Note that using the new import { Client } from '@stellar/stellar-sdk/ContractClient entrypoint requires getting rid of the "build the TS Bindings for both Common JS and ES Modules" behavior we had before. I think this is fine. From TypeScript's moduleResolution documentation:

When compiling a library, you don’t know where the output code will run, but you’d like it to run in as many places as possible. Using "module": "nodenext" (along with the implied "moduleResolution": "nodenext") is the best bet for maximizing the compatibility of the output JavaScript’s module specifiers, since it will force you to comply with Node.js’s stricter rules for import module resolution.

We also don't really need these generated NPM modules to be more flexible than stellar-sdk itself, and it doesn't do the build-to-cjs-and-esm thing.

Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

How come in everything that needs updating for 12rc3 there's no reference to the version at all? When the CLI generates the crate does it have a dependence on a specific, or specific and greater than version of the js-stellar-sdk? It probably should if it doesn't.

@chadoh
Copy link
Contributor Author

chadoh commented May 15, 2024

@leighmcculloch stellar-sdk@12rc3 hasn't been released yet. The package.json files in this repo will be updated to point at the official release once it's available.

@chadoh chadoh force-pushed the feat/ts-bindings/stellar-sdk-12rc3 branch from d5d4a59 to e5cbfc1 Compare May 15, 2024 20:48
@socket-security
Copy link

socket-security bot commented May 15, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@types/node@20.12.12 None +1 2.13 MB types
npm/typescript@5.4.5 None 0 32.4 MB typescript-bot

🚮 Removed packages: npm/@stellar/stellar-sdk@11.3.0, npm/@types/node@20.11.26, npm/typescript@5.4.2

View full report↗︎

@chadoh chadoh force-pushed the feat/ts-bindings/stellar-sdk-12rc3 branch from e5cbfc1 to 8504bc5 Compare May 15, 2024 20:54
Uses the in-progress branch at
stellar/js-stellar-sdk#962, which will be merged
and released as 12rc3 soon. This PR can be updated to use the release at
that point.

Note that using the new `import { Client } from
'@stellar/stellar-sdk/contract` entrypoint requires getting rid
of the "build the TS Bindings for both Common JS and ES Modules"
behavior we had before. I think this is fine. From [TypeScript's
`moduleResolution`
documentation](https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution-for-libraries):

> When compiling a library, you don’t know where the output code will
> run, but you’d like it to run in as many places as possible. Using
> `"module": "nodenext"` (along with the implied `"moduleResolution":
> "nodenext"`) is the best bet for maximizing the compatibility of the
> output JavaScript’s module specifiers, since it will force you to comply
> with Node.js’s stricter rules for import module resolution.

We also don't really need these generated NPM modules to be more
flexible than stellar-sdk itself, and it doesn't do the
build-to-cjs-and-esm thing.
@chadoh chadoh force-pushed the feat/ts-bindings/stellar-sdk-12rc3 branch from 8504bc5 to 3e69106 Compare May 16, 2024 00:52
@chadoh chadoh marked this pull request as ready for review May 16, 2024 01:47
@chadoh chadoh merged commit 783d056 into stellar:main May 16, 2024
@chadoh chadoh deleted the feat/ts-bindings/stellar-sdk-12rc3 branch May 16, 2024 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[TypeError]: XDR Write Error: [object Object] is not a ScVal

3 participants