Support for tree-shaking

[fkuo-stripe]

Is your feature request related to a problem? Please describe.

Stripe apps developers are looking for a tree-shakeable Stripe SDK to help reduce the size of their apps

stripe/stripe-apps#817

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

[anniel-stripe]

Thanks for the feature request, this is something we're actively working on! We're in the process of updating stripe-node to export an ESM package entrypoint, which will allow us to support tree-shaking. We believe there's more refactoring that has to be done past this effort for the package structure to be tree-shakeable, though.

I'll keep this issue updated as we work on adding the ESM entrypoint, and when we start refactoring the package for better tree shaking.

[SmashingQuasar]

@anniel-stripe I am glad to read this, I can't wait for an ESM version of Stripe.
There is another problem that is directly related to tree shaking that I would like to point out whilst we are at it.

Currently, Stripe is exported under a global namespace in TypeScript without any other export. This makes mocking the Stripe library very difficult for unit testing and we have to jump through many hoops and use unsafe practices to make unit tests work properly. It would be highly appreciated if you could stop using namespaces in the future as namespaces do not translate into anything JavaScript can understand. Ideally, simply export native modules without using the export default syntax.
That would greatly improve the quality of the library in my opinion and it would drastically reduce implementation time.

[anniel-stripe]

Hi, sorry for the late update here! An ESM entrypoint was added in stripe-node v11.15.0. In that entrypoint, we still use the export default syntax (code). We haven't started refactoring the package to be tree-shakeable yet, but we're certainly taking suggestions on things to consider when restructuring the library.

Stripe is exported under a global namespace in TypeScript without any other export. This makes mocking the Stripe library very difficult for unit testing and we have to jump through many hoops and use unsafe practices to make unit tests work properly.

Could you elaborate how the global namespace we export in our types contributes to making Stripe hard to mock @SmashingQuasar ? If I understand correctly, the type definition shouldn't have runtime implications. Also, if you can share, it would be useful to see an example of the expected syntax vs. workarounds you have to employ in your unit tests.

We do hope to stop using TypeScript namespaces in the future, likely as part of the effort to move away from defining our types in an ambient module.

[SmashingQuasar]

Hi, sorry for the late update here! An ESM entrypoint was added in stripe-node v11.15.0. In that entrypoint, we still use the export default syntax (code). We haven't started refactoring the package to be tree-shakeable yet, but we're certainly taking suggestions on things to consider when restructuring the library.

Stripe is exported under a global namespace in TypeScript without any other export. This makes mocking the Stripe library very difficult for unit testing and we have to jump through many hoops and use unsafe practices to make unit tests work properly.

Could you elaborate how the global namespace we export in our types contributes to making Stripe hard to mock @SmashingQuasar ? If I understand correctly, the type definition shouldn't have runtime implications. Also, if you can share, it would be useful to see an example of the expected syntax vs. workarounds you have to employ in your unit tests.

We do hope to stop using TypeScript namespaces in the future, likely as part of the effort to move away from defining our types in an ambient module.

Sure, my initial post may have been unclear.

Within the stripe module, you declared a Stripe namespace.
Within this namespace (which is also dreadfully the name of a class but I'll come back to that later), you declared a class CustomersResource.

Now, when you want to mock the stripe module for unit testing purposes, ideally you want to mock the prototype to avoid any possibility of unintentionally calling a non-mocked method. So, instead of instantiating Stripe and then mocking it's property customers, you want to mock CustomersResource.prototype.

Because of how this has all been built and exported, TypeScript will gladly accept the following code:

import { default as Stripe } from 'stripe';

const exampleCustomersResourceStub: { retrieve: SinonStub } = {
  retrieve: stub(Stripe.Stripe.CustomersResource.prototype, 'retrieve')
};

However, this will obviously not work at runtime, because CustomersResource will be undefined.
This is one of the many issues with TypeScript namespaces. TypeScript assumes that if you have exported a namespace, anything that is declared within that namespace, as long as it is not explicitly private, will be exported and accessible. Since namespaces cannot translate into anything in JavaScript, you end up with an uncertainty in your library: Is your code actually going to be exported properly?
This is why (amongst other reasons) I never use TypeScript namespaces. They never work like we think they do and they always cause problems.

Now regarding exporting a class that has the same name as the namespace it causes two problems.
Firstly, it creates confusion within the IDE when trying to find the definition of Stripe. For example VSCode will not know which one to present and will give you a drop-down menu to choose from. That's not the best developer experience you can provide, especially for people relatively new to TypeScript who are unaware of the pitfalls of namespaces.
Secondly, it's not really a good practice because you are creating shadowing within the typing. What I mean is that the Stripe denomination is ambiguous and context-dependent. Again, this is a pitfall to avoid because it can create situations where you don't know if things are attached to the class or the namespace. It's actually the case in what I explained before. CustomersResource seems attached to the Stripe class (which has a reality in JavaScript), whilst in fact, it is attached to the Stripe namespace (which does not have a reality and is unsafe as I explained). This led me to believe my code would work which has ultimately been proven wrong.

I hope this clarifies my original post and gives more insight on TypeScript namespaces.

This is a subjective post obviously, but as a rule of thumb, anything that has no Javascript reality should be exported as export type { } and everything that has a Javascript reality should be exported as export { }. No export default is preferable for tree-shaking and other many pitfalls of the synthetic default exports. Also, it would be preferrable if everything was exported. I know some people prefer type inference but many other don't and not having every type exported is a pain.

One last observation: I've noticed that you have many .d.ts files within your library. Those files really shouldn't be created manually (if they are). This extension is normally reserved for type files created by the TypeScript compiler. As I understand your code base, it seems to be automatically generated but I'm not sure it is actually generated using tsc.

[anniel-stripe]

Thanks for the detailed explanation! This is very helpful to consider moving forward. I see what you mean now and completely agree with you - the TypeScript definitions don't reflect the reality, which negatively impacts the developer experience in cases like this.

You're right that our .d.ts files are automatically generated, but not by tsc. stripe-node's source code was not originally written in TypeScript, so we had to define stripe-node's typings "manually" (not from the TS compiler) as an ambient module declaration. Unfortunately that is still the case, as more work remains to replace these with compiler-emitted typings. Again, moving away from ambient modules and generating types directly from the compiler is being tracked in #1636. As part of this work, we plan to do away with the Stripe namespace for the reasons you mentioned.

[SmashingQuasar]

If you need help to migrate your codebase, to re-evaluate the applicative architecture of the library or to improve TypeScript usage, I would be glad to help.