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

Any reason why DANGEROUS operations like this work and surprisingly does not work in ONLY one edge case? #55763

Closed
abaldawa opened this issue Sep 16, 2023 · 6 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@abaldawa
Copy link

abaldawa commented Sep 16, 2023

πŸ”Ž Search Terms

"generics", "generics type constraints", "generics record index key value change"

πŸ•— Version & Regression Information

  • This is a crash

⏯ Playground Link

https://www.typescriptlang.org/play?noUncheckedIndexedAccess=true#code/PTAEEYDpQZQYwKYDsCGAnAlge1AWlAIKgAiKSA5gmlgK4DOoWADlSgC7ZKgDuAFhnF6gAKjFAoANhKzc6AKABmNJHA5YuAWxpt2CcAAosAIwBWALlAAlBHCxoAJgB46bTBQA0oZQGskMpAB8AJSgAN5yoJGMppDeCACeoAC8EADcoCCgbPwMGAz2ZJTU9NAAcjhU1GhyAL5ycrZILtEm4MlhcfEWAOR0WBoIoJ3dNan1Wjpseoam4EHpmTC8tBL2ImKVdozaoLxUgyi5bKCbaAy0x3zIoEyHdBgU4qBoCCj26hKJbDh+XC9vH3i9UafQkCEg0nIM1asQSkG+ABksHBJAgkdwqABhQ4IfRBeYZMAAUQAGpiiQAFYQASQA8qULJ1QHlQH5uKyaBojFRQKVacJQC43ORoHJMrgJZKpdKZdLQETSsQ8LLcPUxWAAEzQeDIdDYPCgAAS+3WNxeADdkGwGKdQApqBpdigmCwkA9yKAjDt3khusdfDIeLwgUoVGpNNpdBrHAQ0OQbQAPKZIewMay2BzOVzuzw+NmBALQiyx+MhcJRQmgADqwdA7wQDGyLNtFwA-BEosYTLDEilwAswMIcic0FVztptTg+cJDdTSgBxUAorjc0ATXRrPYveoVjAKfTdYbMrhd0AAMjPWXiLCwChaPeSSRS3SQnO5aG6ZY7FcimT5LSGBIGGXeUADUiUsT1BkEQoEDWPxjg0dgpjQLI9ivFhyBodB7GgGt4lbQj2x-TsYiZFINQHUBqWOFwMCkEcql6bZjgeOgmAwKZGDvSQJDQwY2GvBAsJw79QDqOoGnUZouw1dpQk6Ho+gGQD4hGMY5HXKYNWhDV5mBaSsDBCEsChWSe3hLAkRRMF0SxHE8QJTImSrakEQRKDBWzR4t3BUAKQggAxIlMWEdVlRVSLZXlRUIplNUxQAKkSiJEtAABmUVQDS1LQECozpG4d1YEQVBMBwLURGHOgaDQJhMHuChPhIWkiTEaczQQS0kGOW17X6J0XWQd1cp9P0hjZIMgWy4BFGUVRODXSMpnSmM40TZNUysGw7CcIUcy8JAA24QIAG0AF1Cy7Yt1q-Xd9y7U6AAZzrukiWme86Hz7KihxZYhWva-lGLscc2Dw4N4gIsSJIMppji7dL2lOhSEiU-pBmGGpzo0rSEHS6F0v0qSmiM8FIUJz7WwsxFkVROy0GxOhcXxKjSXJKk6QZVTmQYSbXy5HkOv2ihRXFKKJalGKlUi+pMgAVm1Uq9RwfAmwYbg7G8BhDhOBMWFUOCSeaf4fU+WM0AsU3AS84ULuR7oFAwM42G6HG4eaLT3Qtiw838e2UmtpBzdHX7h1OMGlzsF5VE+cLIuluLVXCgAWJXdXKg0QNee5mpgihBiD5quxsf0gON44i-iWlTHk7wLA1GpxGA6S2A0kFEOW90a-MbaMz27zyFzI78wCdoq57sYvYoHvYnaXpB+6KjyhB1DfObngECkeoJ7IyzAowBM4P0PSw5ZVz3JEQ1LFpKsN8PbpedZHBpAL1CUA5QXULZeOJcTyWgA

πŸ’» Code

// 1. Scenario - A Dangerous operation which TS allows
function mutate1(obj: Record<string, unknown>) {
    obj.key = 1; // this is dangerous. No error
}

const obj1 = {key: 'some key'};

mutate1(obj1); // Should TS error out here as it errors out when passing a readonly to non readonly

console.log(obj1.key.toLocaleLowerCase()); // EXCEPTION: key is now number NOT string. 
// -------------------------- END ---------


// 2. Scenario - Here TS prevents error from happening but don't know why
function mutate2<Args extends Record<string, unknown>>(obj: Args) {
    // Why does this error out?
    obj.key = 1; // This errors out. So NOTHING can be mutated here

    if('key' in obj && typeof obj.key === 'number') {
        // NO obj keys can EVER be changed not matter the typeguard. Why????
        obj.key = 2; // It still error's out inspite of all the typeguard
    }
}

const obj2 = {key: 'some key'};

mutate2(obj2);

console.log(obj2.key.toLocaleLowerCase()); // key WILL be string here. PERFECT
// -------------------------- END ---------



/**
 * 3. 
 * 
 * Following Scenario 2. This surprisingly DOES NOT prevent error from happening
 * don't know why
 */
function mutate3<Args extends Record<string, unknown>[]>(obj: Args) {
    if(obj[0]) {
        obj[0].key = 1; // This DOES NOT errors out. Whyyy?
    }
}

const obj3 = [{key: 'some key'}];

mutate3(obj3);

console.log(obj3[0]?.key.toLocaleLowerCase()); // EXCEPTION: key is now number NOT string. 
// -------------------------- END ---------

// 5. Scenario - this works as expected
const readonlyArr: readonly string[] = ['first'];

const mutatingArr: unknown[] = readonlyArr; // This errors out correctly
// --------- END -----

// 4. Scenario - can easily change readonly object keys
const readonlyObj = {k: 2} as const;

const mutatingObj: Record<string, unknown> = readonlyObj;
mutatingObj.k = 'string'; // No error here as well

readonlyObj.k.toFixed(2); // This WILL THROW as 'k' is no longer a number now
// ------------ END ------------

πŸ™ Actual behavior

1] If a function accepts an argument of type Record<string, unknown> then the function can change the value of any key to ANY value. While this makes sense within the function, but, if you pass a well structured object to this method as an argument then the function can change any key to any value and that will result in runtime error and there is no compile time error when a well structured object is passed to a function accepting Record<string, unknown>. But, if you pass a readonly array to non readonly array then it errors out properly.

2] If a function accepts the same argument type Record<string, unknown> via generics then changing ANY keys of the object within the function is not possible.

3] If the function accepts the argument of type Record<string, unknown>[] this time as a array then the function can change any key of any object within the array. This is in stark contrast to point 2 where it is not possible.

4] Readonly array cannot be assigned to non readonly array but readonly object CAN be assigned to non readonly object. Why this difference in functionality?

πŸ™‚ Expected behavior

1] Readonly object should NOT be assignable to Record<string, unknown> as any keys can be mutated within them causing dangerous side effects.

2] The behaviour of Record<string, unknown> should be the same regardless of whether it is enforced via generics or directly as a function argument type.

3] Generics constraint Record<string, unknown>[] i.e. array variant (here any key of any object within the array can be changed) does not seem to behave like generic constraint enforced via Record<string, unknown> i.e. non array variant (here no object keys can be changed)

Additional information about the issue

No response

@MartinJohns
Copy link
Contributor

Duplicate of #13347. Readonly objects are implicitly compatible with the mutable versions.

@abaldawa
Copy link
Author

@MartinJohns I have highlighted many other points besides that just one point. It is not a full duplicate of the #13347, only one point matches with the issues I have highlighted

@whzx5byb
Copy link

whzx5byb commented Sep 16, 2023

This is a well-known unsoundness: A writable property should be invariant but is considered covariant by default. See also #55451 and comments.

And it is very unlikely to be "fixed" because the team treats it as "trade-off" for usability. See more discussions: #9825.

@MartinJohns
Copy link
Contributor

This is a crash

Also, fyi, this is not a crash. "Crash" refers to when the compiler crashes.

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Sep 18, 2023
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Question" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2023
@isti115
Copy link

isti115 commented Sep 23, 2023

@abaldawa The reason why read-only arrays are not assignable to mutable arrays is explained by #13347 (comment):

"... ReadonlyArray has no push method, making it incompatible with Array."

(Note that readonly string[] is just syntax for ReadonlyArray<string>:
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html#a-new-syntax-for-readonlyarray)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

6 participants