-
Notifications
You must be signed in to change notification settings - Fork 142
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
td.replaceEsm() without namedExportStubs throws weird error under windows #475
Comments
Interesting! Other than tagging @giltayar for help, I wonder if this issue also exists under Windows under the latest Node 16.13.0? |
Will look into it. |
Took me a while, but I looked. Unfortunately, the code works under Mac, and I have no Windows machine. I did research it a bit, and when you pass an Not sure what I can do with this. Looks like a bug in the Windows version of Node.js. Maybe open a bug for Node? |
What solved for me, was to do the test.beforeEach(async () => {
//service = await td.replaceEsm('../../'); // moved from here
...
...
service = await td.replaceEsm('../../'); // to here
})
I believe the issue is on starting things.
If we have an `await` before calling `replaceEsm` I think it solves the problem. |
I've been investigating this issue and I believe the root is that in I hacked in a quick fix in
I've spun up a linux vm, and I'm working from there for now to get past this, but I may end up converting my code back to CommonJS. I wasn't aware ESM imports were so finicky when I decided to use them. |
@RexMorgan thanks for looking into this. I think that a PR that used a more durable pattern (or, if Windows always behaves this way, even for network shares, a branching condition on os.platform()) would resolve this issue. Would you be interested in helping us with that? |
@searls Sure, I can take a deep dive into this when I get some down time. I ended up switching back to the commonjs require calls, but I'd love to be able to use ESM imports :) Also, I think this will be a fix for quibble, not testdouble.js |
Any news on this issue? I have a new ES6 hobby project for which I would like to use testdouble, and I ran into exactly the same issue. My development machine is Windows. I am attempting something as simple as
I played around with my own code as well as the the quibble code. Passing in an absolute path in the form of This is really the only post that I can find related to this issue. I am really astounded that there are not more. I mean, are developers with Windows machines using ES6 and modules, and requiring testdouble, really that rare? |
@cwienands1 it's been a few years since I've heard of anyone using Node.js outside Windows Subystem for Linux, for what it's worth, which resulted in me hearing a lot less about Windows-specific issues. Assuming this fails only under the Windows command line? I would like to fix this, I just don't have a development environment set up with Windows. |
I'm no longer developing on Windows and am on macos, now. So, I'm unable to look at this anymore. |
I now have a Windows environment, so if you send me a sample repo reproducing this error, I'll gladly look into this. |
Description
I was evaluating testdouble to do mocking with native ES modules, but I run into a weird error that seems to be a bug.
Issue
it throws
When doing the following:
If I add a replacement object as second argument, it doesn't fail:
await td.replaceEsm('../../src/mynewsdesk.mjs', {});
Looking at the source, if I have 2 or more arguments, it just transparently forwards down to quibble.
Environment
node -v
output: v14.18.1npm -v
(oryarn --version
) output: 6.14.15npm ls testdouble
(oryarn list testdouble
) version: [email protected]Windows 10.
The same code, doesn't throw error if running in Linux, via WSL. Even weirder though is that the code just works, without specifying a loader, however maybe the loader only makes the stubbing works so without an assertion I won't notice it.
The text was updated successfully, but these errors were encountered: