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

tsc --watch regression in Typescript 1.8 Beta #6941

Closed
Jason-Rev opened this issue Feb 6, 2016 · 21 comments
Closed

tsc --watch regression in Typescript 1.8 Beta #6941

Jason-Rev opened this issue Feb 6, 2016 · 21 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue High Priority

Comments

@Jason-Rev
Copy link

With the beta version of 1.8 installed using:

npm install typescript@1.8.0 -g

tsc --watch from project root does not appear to be working. It compiles once but any subsequent changes to the files has no effect.

going back to 1.7.5, everything works fine.

This seems related to #6511.

npm install typescript@next which gets version 1.9.0 also does not seem to be working.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 6, 2016

What version of node are you running? What os? And are any files that are watched correctelly?

@Jason-Rev
Copy link
Author

Good questions:
Node: v5.1.1
OS X 10.10.5
Bash terminal
It seems to be an issue with both .tsx and .ts files.
I wasn't able to get it to detect any changes.

@Jason-Rev
Copy link
Author

Upgrading to Node: v5.5.0 didn't seem to make a difference.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 6, 2016

@billti do you see a similar behavior? i do not see issues on v.5.5.0 on windows.

@billti
Copy link
Member

billti commented Feb 6, 2016

I just installed Node v5.5.0 and created a simple project with no issues. I added a .tsx file, tried with a couple of different "jsx" settings, and tsc -w picked up changes and compiled to my outDir fine.

I'm using the latest OS X (10.11.3) but it seems unlikely this is the cause. Anything beyond a very typical setup that might be causing this? If you create a new super-simple project with only a couple of files do you see the same problem?

@Jason-Rev
Copy link
Author

I don't have an outDir specified:

{
  "compilerOptions": {
    "target": "ES5",
    "module": "commonjs",
    "moduleResolution": "node",
    "sourceMap": true,
    "jsx": "react",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "removeComments": false,
    "noImplicitAny": false,
    "preserveConstEnums": true
  },
  "exclude": [
    "node_modules",
    "bower_components"
  ]
}

I'll see if I can create a small sample project.

@Jason-Rev
Copy link
Author

I have made a project that seems to break: tsc-watch
Note: It is not a working project, is just demonstrates the --watch issue.

Install:

git clone git@github.com:Jason-Rev/tsc-watch.git
cd tsc-watch
npm install
tsd install
tsc -w

Everything should build without warnings or errors.
.js and .map files will be created next to the .ts and .tsx files.

Choose test/test.ts file to edit.
Add a few lines and save.
For me, the changes to test.ts are not picked up. Changes to other files are also not picked up.

Cheers,
Jason

@mhegazy
Copy link
Contributor

mhegazy commented Feb 7, 2016

This project is working fine for me on my Windows box. @vladima can you give tsc--w on Ubuntu a try as well.

@Jason-Rev
Copy link
Author

To be clear, tsc -w picks up new files. But it does not pick up changes to existing files.

@vladima
Copy link
Contributor

vladima commented Feb 8, 2016

Ubuntu 15.10

  • cloned project, npm install & ./node_modules/.bin/tsd install
  • ./node_modules/.bin/tsc -w -p . - tsc is started in watch mode
  • edited existing files test/test.ts and test/actions/actions.ts - all changes were picked up after file is saved
  • added new file newFile.ts at the project root, edited it - worked fine, project is recompiled after saving file

Please let me know if anything is missed or you've done it differently.

@Jason-Rev
Copy link
Author

I wonder if it is a case-sensitive issue. OSX is case-insensitive. I believe Ubuntu is case-sensitive.

I had a brief look at the code that changed around the watcher and it looked like there were different code paths for case-sensitive vs case-insensitive.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 8, 2016

#6511 was a change related to case sensitivity. but that was fixed a while ago. we have not touched this code since. do you see this on other machines? is it all projects? does it happen with no tsconfig.jon (e.g. tsc --w file.ts)?

@geowarin
Copy link

geowarin commented Feb 9, 2016

Same problem here on OSX 10.11.3.
Watch does not pick up any change neither with a tsconfig file nor listing the files in the commandline.
I reproduced the issue with typescript@1.8.0-dev.20160125 and typescript@1.9.0-dev.20160209.

@Jason-Rev
Copy link
Author

I found the issue.
I think it broke when the watch logic between directories and files was separated.

The issue is with how changes are tracked in: sys.ts:fileEventHandler

Here is why I see it and other people are not.
I am using WebStorm as an editor.
When it saves changes to a file, it does not write directly to the file. Instead it:

  1. writes to test.ts___jb_bak___
  2. it then renames test.ts to test.ts___jb_old___.
  3. Once than is done, it renames test.ts___jb_bak___ to test.ts
  4. Because fileEventHandler is only looking at "change" events, it misses out on the fact the file it was watching got renamed and replaced by a new file.

sys.ts

function fileEventHandler(eventName: string, relativeFileName: string, baseDirPath: Path) {
    // When files are deleted from disk, the triggered "rename" event would have a relativefileName of "undefined"
    const filePath = typeof relativeFileName !== "string"
        ? undefined
        : toPath(relativeFileName, baseDirPath, createGetCanonicalFileName(sys.useCaseSensitiveFileNames));
    if (eventName === "change" && fileWatcherCallbacks.contains(filePath)) {
        for (const fileCallback of fileWatcherCallbacks.get(filePath)) {
            fileCallback(filePath);
        }
    }
}

the fix is to correctly handle "rename" events.

Simple Repro steps:

  1. Use the given project
  2. tsc -w in the project root
  3. In a new shell cd test
  4. cat test.ts > test.ts_new
  5. echo "\n// changes" >> test.ts_new
  6. mv test.ts test.ts_old
  7. mv test.ts_new test.ts

Expected that the new test.ts file get compiled.
Actual: nothing happens.

@geowarin
Copy link

geowarin commented Feb 9, 2016

@Jason-Rev Good catch! I'm using IntelliJ.
Changes are indeed correctly picked up when I save the file in a text editor.

@geowarin
Copy link

geowarin commented Feb 9, 2016

Trivially changing the test to

(eventName === "change" || eventName == "rename")

seems to work but also produces a scary error:

2016-02-09 16:03 node[79800] (FSEvents.framework) FSEventStreamFlushSync(): failed assertion '(SInt64)last_id > 0LL'
4:03:40 PM - File change detected. Starting incremental compilation...

@billti
Copy link
Member

billti commented Feb 9, 2016

Looks like a very lengthy write-up of the cause of that exception at https://github.com/bdkjones/fseventsbug/wiki/realpath()-And-FSEvents . (Found by a reference to it on a related Node issue - nodejs/node#854 ).

Will did into this a little more and see what the best solution might be...

@mhegazy mhegazy added Help Wanted You can do this Bug A bug in TypeScript High Priority and removed Help Wanted You can do this labels Feb 9, 2016
@mhegazy mhegazy added this to the TypeScript 1.8.2 milestone Feb 9, 2016
@billti
Copy link
Member

billti commented Feb 12, 2016

I've spent a couple hours on this, and I've been unable to repro the scary FSEvent error when handling the rename operations. The article above does state this is fixed in the latest OS X, which I'm on, so perhaps that's why I can't reproduce it. (Though @geowarin , you state you are on OS X 10.11.3, which is the same as me. Can you confirm you are seeing the FSEvent error on this OS version)?

There was also a bug in how --watch was detecting the compiler options if using a config file, which caused me a bunch of time, but I fixed that also. Pushing the pull request as soon as the tests pass...

@geowarin
Copy link

@billti Yeah, the FSEventStreamFlushSync is probably unrelated. I get it sometimes with typescript in watch mode. I do have the latest OSX version so there is still a bug out there 😄

Reading the article you linked to, it seems to be a random problem with the filesystem so typescript should probably not worry about it. Moreover, the compiler seem to work fine even with the error message showing. Sorry if it has caused confusion.

@Jason-Rev
Copy link
Author

Thank you @billti. I'll verify that your fix worked and close later today.

@billti
Copy link
Member

billti commented Feb 12, 2016

Thanks. This is merged into release-1.8 and master now. Add a comment if you still see any issues (other than the benign OS X message) and I'll reopen.

@billti billti closed this as completed Feb 12, 2016
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Feb 17, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue High Priority
Projects
None yet
Development

No branches or pull requests

5 participants