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.

Sign up for GitHub

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

Do not widen computed properties with template literal types #54706

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Do not widen computed properties with template literal types #54706

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4609a44
Do not widen computed properties with template literal types
Andarist Jun 19, 2023
275410d
Simplify the implementation by using a `Set`
Andarist Jun 19, 2023
6e0b733
Just use `getSymbolLinks`
Andarist Jun 19, 2023
adf7db9
Merge remote-tracking branch 'origin/main' into dont-widen-computed-p…
Andarist Jul 5, 2023
c12e8a7
Merge remote-tracking branch 'origin/main' into dont-widen-computed-p…
Andarist Aug 27, 2023
2284f68
Merge remote-tracking branch 'origin/main' into dont-widen-computed-p…
Andarist Nov 11, 2023
47cb307
add a basic generic test case
Andarist Nov 11, 2023
1a97927
Merge remote-tracking branch 'origin/main' into dont-widen-computed-p…
Andarist May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 32 additions & 25 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30639,20 +30639,27 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
isTypeAssignableToKind(checkComputedPropertyName(firstDecl.name), TypeFlags.ESSymbol));
}

function getObjectLiteralIndexInfo(node: ObjectLiteralExpression, offset: number, properties: Symbol[], keyType: Type): IndexInfo {
function getObjectLiteralIndexInfo(offset: number, properties: Symbol[], keyType: Type, isReadonly: boolean): IndexInfo {
const propTypes: Type[] = [];
for (let i = offset; i < properties.length; i++) {
const prop = properties[i];
if (
keyType === stringType && !isSymbolWithSymbolName(prop) ||
keyType === numberType && isSymbolWithNumericName(prop) ||
keyType === esSymbolType && isSymbolWithSymbolName(prop)
) {
if (keyType === numberType && isSymbolWithNumericName(prop) || keyType === esSymbolType && isSymbolWithSymbolName(prop)) {
propTypes.push(getTypeOfSymbol(properties[i]));
}
else if (!isSymbolWithSymbolName(prop)) {
if (keyType === stringType) {
propTypes.push(getTypeOfSymbol(properties[i]));
}
else {
const source = getSymbolLinks(prop).computedNameType || getStringLiteralType(unescapeLeadingUnderscores(prop.escapedName));
if (isTypeAssignableTo(source, keyType)) {
propTypes.push(getTypeOfSymbol(properties[i]));
}
}
}
}
const unionType = propTypes.length ? getUnionType(propTypes, UnionReduction.Subtype) : undefinedType;
return createIndexInfo(keyType, unionType, isConstContext(node));
return createIndexInfo(keyType, unionType, isReadonly);
}

function getImmediateAliasedSymbol(symbol: Symbol): Symbol | undefined {
Expand All @@ -30675,6 +30682,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const allPropertiesTable = strictNullChecks ? createSymbolTable() : undefined;
let propertiesTable = createSymbolTable();
let propertiesArray: Symbol[] = [];
let indexKeyTypes = new Set<Type>();
let spread: Type = emptyObjectType;

pushCachedContextualType(node);
Expand All @@ -30688,9 +30696,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const isJSObjectLiteral = !contextualType && isInJavascript && !enumTag;
let objectFlags: ObjectFlags = freshObjectLiteralFlag;
let patternWithComputedProperties = false;
let hasComputedStringProperty = false;
let hasComputedNumberProperty = false;
let hasComputedSymbolProperty = false;

// Spreads may cause an early bail; ensure computed names are always checked (this is cached)
// As otherwise they may not be checked until exports for the type at this position are retrieved,
Expand Down Expand Up @@ -30735,7 +30740,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (nameType) {
prop.links.nameType = nameType;
}

else if (computedNameType) {
prop.links.computedNameType = computedNameType;
}
if (inDestructuringPattern) {
// If object literal is an assignment pattern and if the assignment pattern specifies a default value
// for the property, make the property optional.
Expand Down Expand Up @@ -30786,9 +30793,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
spread = getSpreadType(spread, createObjectLiteralType(), node.symbol, objectFlags, inConstContext);
propertiesArray = [];
propertiesTable = createSymbolTable();
hasComputedStringProperty = false;
hasComputedNumberProperty = false;
hasComputedSymbolProperty = false;
indexKeyTypes = new Set();
}
const type = getReducedType(checkExpression(memberDecl.expression, checkMode & CheckMode.Inferential));
if (isValidSpreadType(type)) {
Expand Down Expand Up @@ -30818,16 +30823,19 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
checkNodeDeferred(memberDecl);
}

if (computedNameType && !(computedNameType.flags & TypeFlags.StringOrNumberLiteralOrUnique)) {
if (isTypeAssignableTo(computedNameType, stringNumberSymbolType)) {
if (computedNameType && !isTypeUsableAsPropertyName(computedNameType)) {
if (isPatternLiteralType(computedNameType)) {
indexKeyTypes.add(computedNameType);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably warrants the "breaking change" tag, since previously these'd make a string index signature get added, but now you get a more specific index signature. Which is, I believe, the goal of the change, it's just... maybe breaky? It's unclear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also there's the whole union thing - but the union thing can be a problem for another day.

}
else if (isTypeAssignableTo(computedNameType, stringNumberSymbolType)) {
if (isTypeAssignableTo(computedNameType, numberType)) {
hasComputedNumberProperty = true;
indexKeyTypes.add(numberType);
}
else if (isTypeAssignableTo(computedNameType, esSymbolType)) {
hasComputedSymbolProperty = true;
indexKeyTypes.add(esSymbolType);
}
else {
hasComputedStringProperty = true;
indexKeyTypes.add(stringType);
}
if (inDestructuringPattern) {
patternWithComputedProperties = true;
Expand Down Expand Up @@ -30879,8 +30887,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
spread = getSpreadType(spread, createObjectLiteralType(), node.symbol, objectFlags, inConstContext);
propertiesArray = [];
propertiesTable = createSymbolTable();
hasComputedStringProperty = false;
hasComputedNumberProperty = false;
indexKeyTypes = new Set();
}
// remap the raw emptyObjectType fed in at the top into a fresh empty object literal type, unique to this use site
return mapType(spread, t => t === emptyObjectType ? createObjectLiteralType() : t);
Expand All @@ -30889,10 +30896,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return createObjectLiteralType();

function createObjectLiteralType() {
const indexInfos = [];
if (hasComputedStringProperty) indexInfos.push(getObjectLiteralIndexInfo(node, offset, propertiesArray, stringType));
if (hasComputedNumberProperty) indexInfos.push(getObjectLiteralIndexInfo(node, offset, propertiesArray, numberType));
if (hasComputedSymbolProperty) indexInfos.push(getObjectLiteralIndexInfo(node, offset, propertiesArray, esSymbolType));
const indexInfos: IndexInfo[] = [];
for (const keyType of indexKeyTypes) {
indexInfos.push(getObjectLiteralIndexInfo(offset, propertiesArray, keyType, /*isReadonly*/ inConstContext));
}
const result = createAnonymousType(node.symbol, propertiesTable, emptyArray, emptyArray, indexInfos);
result.objectFlags |= objectFlags | ObjectFlags.ObjectLiteral | ObjectFlags.ContainsObjectOrArrayLiteral;
if (isJSObjectLiteral) {
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5763,6 +5763,7 @@ export interface SymbolLinks {
type?: Type; // Type of value symbol
writeType?: Type; // Type of value symbol in write contexts
nameType?: Type; // Type associated with a late-bound symbol
computedNameType?: Type; // Type of the computed property name not usable as property name
uniqueESSymbolType?: Type; // UniqueESSymbol type for a symbol
declaredType?: Type; // Type of class, interface, enum, type alias, or type parameter
typeParameters?: TypeParameter[]; // Type parameters of type alias (undefined if non-generic)
Expand Down
115 changes: 115 additions & 0 deletions tests/baselines/reference/computedPropertyNamesTemplateLiteralTypes.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
computedPropertyNamesTemplateLiteralTypes.ts(94,7): error TS2322: Type '{ [x: string]: string | number[]; name: string; }' is not assignable to type 'IDocument_46309'.
'string' and '`added_${string}`' index signatures are incompatible.
Type 'string | number[]' is not assignable to type 'number[] | undefined'.
Type 'string' is not assignable to type 'number[]'.


==== computedPropertyNamesTemplateLiteralTypes.ts (1 errors) ====
declare const str1: string;
declare const pattern1: `foo${string}`;
declare const pattern2: `foobar${string}`;
declare const samepattern1: `foo${string}`;

const obj1 = {
[pattern1]: true,
};

const obj2 = {
[pattern1]: true,
[str1]: 100,
};

const obj3 = {
[str1]: 100,
[pattern1]: true,
};

const obj4 = {
[pattern1]: true,
[pattern2]: "hello",
};

const obj5 = {
[pattern2]: "hello",
[pattern1]: true,
};

const obj6 = {
[pattern1]: true,
[pattern2]: "hello",
other: 100,
};

const obj7 = {
[pattern1]: true,
[pattern2]: "hello",
fooooooooo: 100,
};

const obj8 = {
[pattern1]: true,
[pattern2]: "hello",
foobarrrrr: 100,
};

const obj9 = {
[pattern1]: true,
[samepattern1]: "hello",
};

const obj10 = {
[pattern1]: true,
} as const;

const obj11 = {
[pattern1]: 100,
...obj9,
};

const obj12 = {
...obj9,
[pattern1]: 100,
};

const obj13 = {
[pattern1]: 100,
...{
[pattern2]: "hello",
},
};

const obj14 = {
[pattern1]: 100,
...{
[pattern1]: true,
[pattern2]: "hello",
foobarrrrr: [1, 2, 3],
},
};

// repro from https://github.com/microsoft/TypeScript/issues/46309

interface IDocument_46309 {
name: string;
[added_: `added_${string}`]: number[] | undefined;
}

const tech1_46309 = {
uuid: "70b26275-5096-4e4b-9d50-3c965c9e5073",
};

const doc_46309: IDocument_46309 = {
~~~~~~~~~
!!! error TS2322: Type '{ [x: string]: string | number[]; name: string; }' is not assignable to type 'IDocument_46309'.
!!! error TS2322: 'string' and '`added_${string}`' index signatures are incompatible.
!!! error TS2322: Type 'string | number[]' is not assignable to type 'number[] | undefined'.
!!! error TS2322: Type 'string' is not assignable to type 'number[]'.
name: "",
[`added_${tech1_46309.uuid}`]: [19700101],
};

const doc2_46309: IDocument_46309 = {
name: "",
[`added_${tech1_46309.uuid}` as const]: [19700101],
};

Loading