Skip to content

LS#getEmitOutput misses constant enum values on second emit #1599

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

Closed
jrieken opened this issue Jan 5, 2015 · 4 comments
Closed

LS#getEmitOutput misses constant enum values on second emit #1599

jrieken opened this issue Jan 5, 2015 · 4 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@jrieken
Copy link
Member

jrieken commented Jan 5, 2015

  • have two external modules (a.ts which defines and exports an enum, b.ts which uses the enum)
  • emit the JS code for b.ts
  • emit the JS code for a.ts and notice that the enum reference is inlined (console.log(0 /*A*/);)
  • change a.ts and emit only that file again
  • => now the enum is not inlined anymore (console.log(Foo.A);). I can debug this into checker#getConstantValue which discovers the enum value but fails to find a proper node link. I guess this is because changing a.ts cleared some type caches)

a.ts

import Foo = require('./b');
console.log(Foo.A);

b.ts

enum Foo {
    A = 0,
    B = 1,
    C = 2
}

export = Foo;
@mhegazy
Copy link
Contributor

mhegazy commented Jan 8, 2015

this is by design, as we do not track references to const enums. you will also run into the same issue with module properties:

a.ts

module m {
     export var x = 0;
}
var x = "global";

module m {
    console.log(x); // m.x, and not global x.
}

Now if you call getEmitOutput on a.ts, then b.ts then edit a.ts to remove the export for x, b.js still references an undefined m.x.

This is ok if you do not rely on incremental build as the only build system. I think we need a proposal for this.

@mhegazy mhegazy added the Bug A bug in TypeScript label Jan 8, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.5, TypeScript 1.6 Jan 8, 2015
@mhegazy mhegazy added Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. and removed Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Jan 8, 2015
@mhegazy mhegazy self-assigned this Jan 15, 2015
@mhegazy mhegazy assigned vladima and unassigned mhegazy Feb 3, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Feb 3, 2015

@vladima should have a flag for this.

@CyrusNajmabadi
Copy link
Contributor

"This is ok if you do not rely on incremental build as the only build system"

I disagree. I don't think it's ever been 'by design' that Incremental Build produce different output than the normal build. It's not acceptable for our incremental approach to result in different code that can then be broken or be hard to track down. As such, i view this purely as a bug.

We already aim for 100% correctness with Incremental Parsing, and we also aim for 100% correctness with --watch compilation. Incremental compile should be at that same bar as well.

@vladima
Copy link
Contributor

vladima commented Mar 19, 2015

This issue should be resolved with 79272d7 - now regular enums are never inlined and always emitted as property access

@vladima vladima closed this as completed Mar 19, 2015
@vladima vladima added the Fixed A PR has been merged for this issue label Mar 19, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants