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

replaceEsm not working on windows #491

Closed
3 tasks
connorjclark opened this issue Jul 1, 2022 · 5 comments
Closed
3 tasks

replaceEsm not working on windows #491

connorjclark opened this issue Jul 1, 2022 · 5 comments

Comments

@connorjclark
Copy link

Description

ES modules are not mocked on windows. Works fine on Mac.

Environment

  • node -v output: 14.8.2
  • yarn --version output: 1.22.19
  • npm ls testdouble (or yarn list testdouble) version: 3.16.15

Example Repo

https://github.com/connorjclark/testdouble-win-example

// main.js
import * as td from 'testdouble';

td.replaceEsm('./module.js', {runMe: () => 'replaced'});

const {runMe} = await import('./module.js')

console.log(runMe());
// module.js
export function runMe() {
    return 'original';
}

node --loader=testdouble main.js prints original on windows, replaced on Mac.

@searls
Copy link
Member

searls commented Jul 3, 2022

Paging @giltayar if he might have any insight 🙏

@connorjclark
Copy link
Author

connorjclark commented Jul 3, 2022

image

The key in the global modules Map duplicated C:/ for some reason. Also, the slashes are wrong.

Set here:

image

I believe convertUrlToPath is eating the C: portion of the module url (interprets as the protocol), and spits out the path without it.

image

@connorjclark
Copy link
Author

connorjclark commented Jul 3, 2022

I made the following change inside quibble, which resolved the bug:

image

if (process.platform === 'win32') {
    fullModulePath = callerFile.replace('/C:', 'C:')
    fullModulePath = path.resolve(path.dirname(fullModulePath), importPath)
    fullModulePath = '/' + fullModulePath.split(path.sep).join("/")
  }

Of course, I shouldn't be hardcoding the drive there. But anyway, the real fix is probably something to do with correctly juggling the format of the Error filename / module name / URLs being passed around, being mindful of Windows's differing slashes / existence of a drive suffix.

@searls
Copy link
Member

searls commented Jul 5, 2022

Thanks for digging into this. Ultimately, if you (or another developer that uses Windows) might consider sending a PR with a test for this that they can assert works OK on windows without breaking anything else, I'd be eager to accept it.

@giltayar
Copy link
Collaborator

Cloned your repo, updated testdouble to the latest version, and added an await before td.replaceEsm. Got the correct result ("replaced"), so closing this issue.

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

3 participants