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.

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

[SDK Refactoring] ESM Modules #514

Open
yagopv opened this issue Aug 29, 2023 · 1 comment · May be fixed by #1049
Open

[SDK Refactoring] ESM Modules #514

yagopv opened this issue Aug 29, 2023 · 1 comment · May be fixed by #1049

Comments

@yagopv
Copy link
Member

yagopv commented Aug 29, 2023 •
edited
Loading

Context / issue

Using current node engines with esm modules is difficult and require some hacks as we are compiling safe-core-sdk using commonjs

With esm, import and export declarations are determined statically at compile time. This static structure allows for better tooling with things like tree shaking (dead code elimination), faster lookup, and advanced optimizations.

Consequences of supporting only commonjs

  • In Browsers: Browser based applications that want to use out SDK will need to transpile the code, which adds an additional step to their build process
  • Compatibility: Modern libraries and applications that prefer or require esm modules might face difficulties integrating with our SDK
  • Tree Shaking: esm enables effective tree shaking, which is important for reducing the final bundle size in frontend applications. commonjs does not support tree shaking efficiently.

By offering both formats, we can ensure wider compatibility and make the SDK more attractive to a broader range of developers and projects.

Proposed solution

The proposed solution would compile safe-core-sdk with a dual output with both commonjs and esm modules being distributed inside the package

Additional context

First issue
Similar solution in safe-apps-sdk
Issue with Safe.create
This was referenced Aug 29, 2023
Closed
Closed
@mattcasey
Copy link

I think I ran into a related bug: when using dynamic imports with this lib, we get a nested 'default' prop. To illustrate:

export async function getSafeApiClient({ chainId }: { chainId: number }): Promise<ISafeApiKit> {

  const imported = await import('@safe-global/api-kit');
  const SafeApiKit = imported.default.default || imported.default;
}

And it depends on how we're running it, so it could be our configuration. If I run the code with ts-node or Next.js, the issue does not happen. If I use esbuild or tsx, then it is nested inside an extra default prop

@yagopv yagopv changed the title feat: ESM Modules [SDK Refactor] ESM Modules Jun 17, 2024
@yagopv yagopv changed the title [SDK Refactor] ESM Modules [SDK Refactoring] ESM Modules Jun 17, 2024
@yagopv yagopv linked a pull request Nov 18, 2024 that will close this issue
chore: build kits with cjs and esm targets #1049
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: build kits with cjs and esm targets safe-global/safe-core-sdk
2 participants
@mattcasey @yagopv