Skip to content

Should treat const and enum (at least const enum) value the same as literals for indexers. #3411

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
vilicvane opened this issue Jun 8, 2015 · 8 comments
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@vilicvane
Copy link

const DATA_END_INDEX = 2;

enum OptimizedDataIndex {
    start,
    data,
    end
}

interface OptimizedData {
    0: number;
    1: any[];
    2: number;
}

var data: OptimizedData;
var start = OptimizedData[OptimizedDataIndex.start];
var end = OptimizedData[DATA_END_INDEX];
// expecting no error. `start` and `end` should be numbers.

This should also apply to something like tuples directly written as [TypeA, TypeB, ...].

@danquirk
Copy link
Member

danquirk commented Jun 8, 2015

Presumably your example intended to index into data and not the interface type. This is sort of the simplest case of constant propogation/flow control since the compiler can at least assume the name<->value pairs here are immutable.

@danquirk danquirk added the Suggestion An idea for TypeScript label Jun 8, 2015
@vilicvane
Copy link
Author

So can I take that as "this suggestion is reasonable"? :D

@RyanCavanaugh RyanCavanaugh added the In Discussion Not yet reached consensus label Jun 10, 2015
@RyanCavanaugh
Copy link
Member

Summary: When we see a property access where the indexing expression is an enum member, we should resolve the property access as if the indexing expression were actually the computed value of the enum member's value.

In other words, var x = Foo[E.bar] is equivalent to var x = Foo[2] if we can resolve E.bar to its value 2.

@vilicvane
Copy link
Author

And also consts, I think.

@jbondc
Copy link
Contributor

jbondc commented Aug 28, 2015

Added a patch for the const enum access. Would also like to see these work:

const one = 1;
module dot {
   export const one = 1; 
}
const enum numbers {
    one = 1
}
const oneRef = one;

interface OptimizedData {
    0: number;
    1: any[];
    2: number;
}
let data:OptimizedData 

// works
data[1] // any[]
data[numbers.one] // any[]

// should work
data[one] // any[]
data[dot.one] // any[]
data[oneRef] // any[]

The last 3 could work but require better tracking of constants and constant propagation like @danquirk mentioned. Don't think you need flow control to do it properly, have gotten it working without anyways.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". and removed In Discussion Not yet reached consensus labels Oct 5, 2015
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Oct 5, 2015
@RyanCavanaugh
Copy link
Member

Approved for const enum members only. @jbondc can you get your PR up-to-date?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 5, 2015

@jbondc, we would appreciate it if you can keep it only for const enums, and not for generic constant propagation.

@mhegazy mhegazy closed this as completed Oct 8, 2015
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Oct 8, 2015
@mhegazy mhegazy modified the milestones: TypeScript 1.7, Community Oct 8, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Oct 8, 2015

thanks @jbondc!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Fixed A PR has been merged for this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants