safe-global / safe-core-sdk

The Safe{Core} SDK allows builders to add account abstraction functionality into their apps.
https://docs.safe.global/sdk/overview
MIT License
253 stars 197 forks source link

[SDK Refactoring] ESM Modules #514

Open yagopv opened 1 year ago

yagopv commented 1 year ago

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

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

mattcasey commented 5 months ago

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