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

feat: SDK POC #11

Merged
merged 29 commits into from
Jun 24, 2024
Merged

feat: SDK POC #11

merged 29 commits into from
Jun 24, 2024

Conversation

BeroBurny
Copy link
Collaborator

Draft, POC

Copy link
Member

@mpetrunic mpetrunic left a comment

Choose a reason for hiding this comment

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

I think it would make sense to intrudce custom error classes for handling errors

"https://gopher.test.buildwithsygma.com/"
);
export function setBaseUrl(url: string): void {
BASE_URL = url;
Copy link
Member

Choose a reason for hiding this comment

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

why not set process.env instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

and you end up in vite that does not to useprocess.env but there is import.meta.env, made helper method to solve this problem and the setter is in a case you have no options to provide .env like in React you need use prefix REACT_ for env's

if (whitelistedSourceChains && whitelistedSourceChains.length)
url.searchParams.set(
"whitelistedSourceChains",
whitelistedSourceChains.toString()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
whitelistedSourceChains.toString()
whitelistedSourceChains.join(",")

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is the same result

const a = [1,2,3,4];

a.join();
// '1,2,3,4'

a.join(',');
// '1,2,3,4'

a.toString();
// '1,2,3,4'

Copy link
Contributor

Choose a reason for hiding this comment

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

but you're using one extra character! 🤣

Copy link
Member

@mpetrunic mpetrunic Jun 24, 2024

Choose a reason for hiding this comment

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

I was going to argue it's more expressive 😂

Copy link
Collaborator Author

@BeroBurny BeroBurny Jun 24, 2024

Choose a reason for hiding this comment

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

I wanted but been layz to make benchmarks.

But it is on mu super "short" TODO list for personal projects

web/src/lib/utils.ts Outdated Show resolved Hide resolved
Comment on lines 8 to 14
const noNetworkFound: Chain = {
chainID: 0,
chainType: 'evm',
name: 'none',
logoURI: 'https://static.thenounproject.com/png/75231-200.png',
rpcurls: []
};
Copy link
Contributor

Choose a reason for hiding this comment

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

same as last comment, just an observation, maybe satisfies would be more appropriate here

Suggested change
const noNetworkFound: Chain = {
chainID: 0,
chainType: 'evm',
name: 'none',
logoURI: 'https://static.thenounproject.com/png/75231-200.png',
rpcurls: []
};
const noNetworkFound = {
chainID: 0,
chainType: 'evm',
name: 'none',
logoURI: 'https://static.thenounproject.com/png/75231-200.png',
rpcurls: []
} satisfies Chain;

packages/sdk/src/api.ts Outdated Show resolved Hide resolved
packages/sdk/src/index.ts Outdated Show resolved Hide resolved
whitelistedSourceChains
);
return Promise.resolve([]);
const tokenList = tokens || (await this.getAvailableTokens());
Copy link
Contributor

Choose a reason for hiding this comment

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

should we getAvailableTokens if provided tokens array is empty too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This tricky question is like if you set an intentionally empty array (it is not logical to do) as an optional parameter and you got the result that is not part that reflects that array.

variable = env[envName];
}
} finally {
variable ??= defaultValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

??= 😍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

packages/sdk/src/types.ts Outdated Show resolved Hide resolved
export type ChainID = number;

export interface FungibleToken {
addresses: { [chainID: ChainID]: Address };
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this can also be a Record<ChainID, Address>, but no need to update

packages/sdk/src/types.ts Outdated Show resolved Hide resolved
@BeroBurny BeroBurny marked this pull request as ready for review June 24, 2024 14:49
@BeroBurny BeroBurny merged commit 6aca865 into master Jun 24, 2024
2 checks passed
BeroBurny pushed a commit that referenced this pull request Sep 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.1.0](sprinter-sdk-v0.0.1...sprinter-sdk-v0.1.0)
(2024-09-12)


### Features

* api options
([#33](#33))
([ad1cb88](ad1cb88))
* cross chain contract call
([#21](#21))
([69c0128](69c0128))
* depricate provider param from sdk
([#41](#41))
([72297fe](72297fe))
* docs POC ([#14](#14))
([8cd800d](8cd800d))
* erc20 with contract call
([#39](#39))
([77e1d8d](77e1d8d)),
closes [#38](#38)
* native tokens transfers
([#40](#40))
([d4edf35](d4edf35)),
closes [#36](#36)
* SDK POC ([#11](#11))
([6aca865](6aca865))
* web poc (super basic basic)
([#3](#3))
([25a91f7](25a91f7))


### Bug Fixes

* process approvals from quote
([#26](#26))
([95d9ebf](95d9ebf))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@BeroBurny BeroBurny deleted the beroburny/sdk-poc branch September 23, 2024 16:39
@github-actions github-actions bot mentioned this pull request Sep 23, 2024
BeroBurny pushed a commit that referenced this pull request Sep 23, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>sprinter-sdk: 0.3.0</summary>

##
[0.3.0](sprinter-sdk-v0.2.0...sprinter-sdk-v0.3.0)
(2024-09-23)


### Features

* api options
([#33](#33))
([ad1cb88](ad1cb88))
* cross chain contract call
([#21](#21))
([69c0128](69c0128))
* depricate provider param from sdk
([#41](#41))
([72297fe](72297fe))
* docs POC ([#14](#14))
([8cd800d](8cd800d))
* erc20 with contract call
([#39](#39))
([77e1d8d](77e1d8d)),
closes [#38](#38)
* implement tracing url resolver
([#46](#46))
([efd1be3](efd1be3)),
closes [#44](#44)
* native tokens transfers
([#40](#40))
([d4edf35](d4edf35)),
closes [#36](#36)
* react sdk context
([#48](#48))
([39dbe7e](39dbe7e)),
closes [#47](#47)
* SDK POC ([#11](#11))
([6aca865](6aca865))
* web poc (super basic basic)
([#3](#3))
([25a91f7](25a91f7))


### Bug Fixes

* process approvals from quote
([#26](#26))
([95d9ebf](95d9ebf))
</details>

<details><summary>sprinter-react: 0.2.0</summary>

##
[0.2.0](sprinter-react-v0.1.0...sprinter-react-v0.2.0)
(2024-09-23)


### Features

* react sdk context
([#48](#48))
([39dbe7e](39dbe7e)),
closes [#47](#47)


### Bug Fixes

* cd and rect hooks export
([#52](#52))
([33c0604](33c0604))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants