-
Notifications
You must be signed in to change notification settings - Fork 32
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
Watch not working on windows #65
Comments
Note: a quick-and-dirty change in - this.emitter._fileList.changeFile(path, true)
+ this.emitter._fileList.changeFile(path.replace(/\\/g, '/'), true) …makes it work on my system. That would however break projects that actually use backslashes in file names on posix systems. Perhaps check the value of |
Something like this? const sep = require('path').sep
//...
this.emitter._fileList.changeFile(sep !== '/' ? path.replace(/\\/g, '/') : path, true) |
@jlmakes path.normalize might work here. Not at my computer, so cannot double check. |
Took a look, and my above comment with Take a look instead at slash.
|
It's basically a wrapper around the same string replace: // slash.js
'use strict';
module.exports = path => {
const isExtendedLengthPath = /^\\\\\?\\/.test(path);
const hasNonAscii = /[^\u0000-\u0080]+/.test(path);
if (isExtendedLengthPath || hasNonAscii) {
return path;
}
return path.replace(/\\/g, '/');
}; I understand testing for ASCII, but I had to search for "extended length paths": |
I know, was just thinking that it could potentially handle the edge cases for you if they ever came up. |
I'm generally cautious of adding dependencies, but I'm not opposed to it. I just don't fully understand if those cases are overly defensive for Karma and Rollup. As in, will non-ASCII or extended length paths occur in a JavaScript code base, or the build process therein?
This would still need to be addressed in the implementation. So, something like this? const slash = require('slash')
const sep = require('path').sep
//...
this.emitter._fileList.changeFile(sep !== '/' ? slash(path) : path, true) |
Just want to point out that |
Good news and bad news! Bad news: 5668d68 does not work here. Good news: the fix in it works.
So, the fix works, but 7.0.4 introduced another issue. I'll open a separate topic. |
Using version 7.0.3.
Past the initial preprocessing, no file change ever trigger test again.
I traced the issue to the matching in the watcher. It passes down changed file path to Karma's fileList. In turn, the fileList does this:
The issue is:
path
is a windows path, in my case,D:\home\projects\skeleton\skeleton-lib\tests\index.js
.file.originalPath
is not:'D:/home/projects/skeleton/skeleton-lib/tests/index.js'
.It looks like Karma's
fileList
expects the given path to always be posix format, andkarma-rollup-preprocessor
sends windows format instead.The text was updated successfully, but these errors were encountered: