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

Elaborate on "For the stubbing to work, the stubbed method cannot be destructured, neither in the module under test nor in the test." #2552

Open
DanKaplanSES opened this issue Sep 30, 2023 · 7 comments

Comments

@DanKaplanSES
Copy link
Contributor

DanKaplanSES commented Sep 30, 2023

Is your feature request related to a problem? Please describe.
The How to stub a dependency of a module documentation says:

To stub a dependency (imported module) of a module under test you have to import it explicitly in your test and stub the desired method. For the stubbing to work, the stubbed method cannot be destructured, neither in the module under test nor in the test.

I'm not really sure what this bold part means. Does it mean you can't destructure the import statement? If so, while I have found that to be true in the test file, I haven't found that the be true in the module under test. That makes me think I'm either:

  1. Misunderstanding what this sentence means.
  2. Reading out of date documentation.

Describe the solution you'd like
I'd like this sentence to be rewritten or expanded upon to clarify. If possible, an example would go a long way.

Describe alternatives you've considered

Additional context

@fatso83
Copy link
Contributor

fatso83 commented Oct 3, 2023

Hi, @DanKaplanSES . I totally get that this is confusing. I have gone into depth on StackOverflow in explaining this a couple of times, one of which is this answer.

That should answer most questions you would have.

I could end it here, but maybe you - based on that - could suggest some rewording and/or example yourself, pushing a PR with the edited text? I am somewhat non-neutral about what I think of the text 😉

@fatso83
Copy link
Contributor

fatso83 commented Oct 3, 2023

Does it mean you can't destructure the import statement?
Yes, in the CommonJS (CJS) world, which is what the world looked like when that article was written. All of it applies to that restriction.

Not in the test, because how would you modify the export of the dependency without an object to modify the export on?
Not in the SUT, because if you bind the reference, any changes to the export will not be affected.

BUT there's a catch
The world has moved on to EcmaScript Modules (ESM), and they are a bit of a different story. How they work is a bit of a mindbender, and it might explain what you are seeing, if you are using native ESM or ESM transpiled to CJS following the ESM spec. If you transpile ESM to a CJS target, you will probably see that the exports are in fact getters (accessor functions), meaning they are immutable.

All in all, this subject validates an update in this day and age, as the rules of the game have changed a bit with ESM.

Regarding this:

If so, while I have found that to be true in the test file, I haven't found that the be true in the module under test.

I am not sure how you can destructure an import in the SUT and have Sinon's modification to the dependency take effect in the SUT. Some actual code showing what you think is false would go a long way. Explaining this was way easier in the world of CJS 😓

@DanKaplanSES
Copy link
Contributor Author

Hi @fatso83 ,

Thanks for the reply. I should've mentioned I'm writing typescript that transpiles into "esnext". Here's an example project: https://stackblitz.com/edit/typescript-svngnu?file=mytest.spec.ts You can't run the tests, but if you look at files a.ts, b.ts and mytest.spec.ts, you'll see that a.ts destructures the b.ts import and the test (should) pass. At least, I write examples like that all the time.

The docs say, "...neither in the module under test nor in the test," but isn't this an example of destructuring in the module?

I'd love to submit a PR after I understand this detail.

@fatso83
Copy link
Contributor

fatso83 commented Oct 3, 2023

Hi, that example is not really runnable, as you assume different things in the test and the SUT. In the b.ts module you expose an object as export b, which has a method called method, whereas in the test you assume that b is the method that you are replacing.

It's hard to discuss non-runnable code, as it makes what we are talking about ambiguous.

Anyway, if you use modules like b.ts, then you are doing something quite different than what is talked about. Usually, you want to directly affect the exports of the modules you are working on. If you manipulate b.method, that will work fine even if you have destructured the reference to b, but then you are not stubbing any exports of the module, rather you are stubbing a field on an export of said module.

P.S. Please do read the answer I linked to. It should clarify some things.

Copy link

stale bot commented Dec 27, 2023

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Dec 27, 2023
@fatso83
Copy link
Contributor

fatso83 commented Sep 10, 2024

@DanKaplanSES I just wonder if I could help this move? Did you figure out what you wanted to clarify? Not sure if I just made everything more confusing 🙈

P.S. I wrote an elaborate guide on various strategies for replacing dependencies in real-world projects using a typical bundler and transpilation steps. You might want to check it out: https://sinonjs.org/how-to/typescript-swc/

@stale stale bot removed the wontfix label Sep 10, 2024
@DanKaplanSES
Copy link
Contributor Author

DanKaplanSES commented Sep 15, 2024

Hi @fatso83 Apologies for being so unresponsive. I've read the StackOverflow post but I haven't read your blog article yet (but will).

In this thread, I think this is the crux of the issue:

if you use modules like b.ts, then you are doing something quite different than what is talked about.

It sounds like I think the documentation is saying one thing but it's really saying another. Before I continue, here's the documentation again with a smaller portion emphasized:

To stub a dependency (imported module) of a module under test you have to import it explicitly in your test and stub the desired method. For the stubbing to work, the stubbed method cannot be destructured, neither in the module under test nor in the test.

If that emphasized portion isn't referring to something like b.ts's method(), what is it referring to? That's what still confuses me. If the part I emphasized is demonstrated in your original StackOverflow answer, can you tell me which line of code it represents?

If the documentation cut out the emphasized part, I wouldn't be confused, because I know what that's referring to and I've experienced it firsthand:

To stub a dependency (imported module) of a module under test you have to import it explicitly in your test and stub the desired method. For the stubbing to work, the stubbed method cannot be destructured in the test.

So, to summarize, I feel like I know how to use sinon correctly, I just don't understand this tiny part of the documentation.

Thanks!

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

No branches or pull requests

2 participants