clausreinke / typescript-tools

(repo no longer active) Tools related to the TypeScript language
Apache License 2.0
266 stars 29 forks source link

tss 1.4: test for undefined instead of null #47

Closed Phaiax closed 9 years ago

Phaiax commented 9 years ago

Hey

  private updateScript(fileName: string, content: string) {
      var script = this.fileNameToScript[fileName];
      if (script !== null) {
        script.updateContent(content);
      } else {
        this.fileNameToScript[fileName] = new harness.ScriptInfo(fileName, content);
      }
      this.snapshots[fileName] = new harness.ScriptSnapshot(this.fileNameToScript[fileName]); 
  }

There are some occasions where you test script !== null, but

> var fn = {}
> fn
{}
> fn['ert']
undefined
> var k = fn['ert']
> k !== null
true
> k !== undefined
false

But ^^

> k == undefined
true
> k == null
true
> 

I can make a pull request if it is easier for you

Phaiax commented 9 years ago

.. i will do a pr and i will do some other stuff :D

Phaiax commented 9 years ago

-> #49

clausreinke commented 9 years ago

Yes, there are a few issues that slipped in when I rewrote the source to match the github version of TS. This particular one comes from a clash of styles: TS tests for null but uses a lookup helper that delivers null instead of undefined, I skipped the lookup helper but didn't adapt the test (I prefer testing for truthiness).

I'll look through your pull request later this week (scheduled to cleanup the mess I made in the rush to get functionality working;-), but for these kind of slipups, bug reports (with reasoning, or with test case) will be sufficient, and welcome.

clausreinke commented 9 years ago

Fixed, using (script) and (!script). This exposes a TS (usage) issue (getSyntacticDiagnostics on a new file throws) MicroSoft/TypeScript#2287, although adding sources that do not exist in the file system is not fully supported in tss anyway.