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

noPropertyAccessFromIndexSignature does not affect object literals #46435

Open
concavelenz opened this issue Oct 19, 2021 · 7 comments
Open

noPropertyAccessFromIndexSignature does not affect object literals #46435

concavelenz opened this issue Oct 19, 2021 · 7 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@concavelenz
Copy link

Bug Report

πŸ”Ž Search Terms

noPropertyAccessFromIndexSignature

πŸ•— Version & Regression Information

4.2+

  • This is the behavior in every version I tried

⏯ Playground Link

https://www.typescriptlang.org/play?noPropertyAccessFromIndexSignature=true#code/FAGwpgLgBBYM4QFxQN4G0CWA7AJmAHoggE7YDmAukRKVmQL5QC8qwU7UA9J1AO5h8AhlmgQAFhjgwA9lABGAgMaCArmTHQ5ATyhZpABWLSADmGIQtAQUWL4cAGJGAtgElcBAMoYyWQRBXEYAA08irQGOFwWADkEAB0bByCcorI0RLRQYnseABmaWJgICDSmcD0ANzAwIrSWHDS4HElZAAUsAhxyYoAlFW19Y1gzdJtHfF5fTV1DU0t7fAQaNHd0RRTA7PD8+PLeWt9QA

πŸ’» Code

let test: {[index:string]:string} = {
    // we want this to be caught by noPropertyAccessFromIndexSignature, but it isn't.
    abc: 'hi',
    def: 'hello',
};

console.log(test.abc); // reported
console.log(test.def); // reported
console.log(test['abc']); // OK
console.log(test['def']);  // OK

πŸ™ Actual behavior

Unquoted string keys are allowed when assigned to types with index signatures.

πŸ™‚ Expected behavior

Some mechanism for restricting the definitions of object literals so that only named properties are allowed to be provided without quotes ( {['somekey']: value} or {'somekey':value}. This would both prevent logic errors and improve compatibility with Closure Compiler's ADVANCED optimizations.

@andrewbranch
Copy link
Member

This would both prevent logic errors and improve compatibility with Closure Compiler's ADVANCED optimizations.

Can you expound on each of these motivations?

@concavelenz
Copy link
Author

I believe for logic errors, I believe this has the same motivations as noPropertyAccessFromIndexSignature. For Closure Compiler's ADVANCED optimization mode, it differentiates between rename-able (optimizable) properties based on whether the property is "quoted". Quoted and computed properties are not renamed or eligible for optimization but "." access/unquoted properties are. noPropertyAccessFromIndexSignature is helpful here as it encourage consistent use of "quoting". One of the teams we support is looking for the some degree of protection for object literals assigned to types with index signatures.

https://developers.google.com/closure/compiler/docs/limitations#implications-of-global-variable,-function,-and-property-renaming:

Closure Compiler as you may know is used heavily at Google.

@andrewbranch andrewbranch added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Oct 20, 2021
@RussellSprouts
Copy link

Similarly, object destructuring assignment is similar to property access, I would expect that destructuring an index signature would also give an error with noPropertyAccessFromIndexSignature:

type Dictionary = Record<string, string>;

let test: Dictionary = {
    // we want this to be caught by noPropertyAccessFromIndexSignature, but it isn't.
    abc: 'hi',
    def: 'hello',
};

// This is equivalent to getting test.abc, so should be an error
const {abc} = test;
// This is preferred
const {'abc': abc1} = test;

https://www.typescriptlang.org/play?noPropertyAccessFromIndexSignature=true#code/FAFwngDgpgBAIgSwMYgQewHYEMBOYYC8MASlEmjgCYA8AziDghgOYA0M9jLAfANzDAANlBAwQUegC54yVJlz4iAb2Aw1MAPQaYAd1g6sGUSAAWCWmLQwARrCRYArsxOjr+DGgAKONNBzgAQSQkCVoAMR8AWwBJDEooAA8AZQRmbBAHHCh2awdRBHzaDAByEAA6VXUsayRpYrNi1kq1eIAzOpMoQUE0RuAAX35gLRgAFTMLcxgoAEcHBAA3LGEjSzEJcuqkdloTNAdBShtYQ2mcHxxgcgx6GCUt-sJ1+n5r26VireLpLYBGR6I4heAjeaGEZR6zAAFEDNjUAJSvTC0MFQCFoaGwsptRFXZGo9GYjYAbU+NWKAF1caDwZCYSTim1KYigA

The Closure Compiler uses the quoting for all of these syntaxes to determine if the property name will be renamed.

@concavelenz
Copy link
Author

@andrewbranch Where would I find the justification for noPropertyAccessFromIndexSignature?

@andrewbranch
Copy link
Member

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-2.html#--nopropertyaccessfromindexsignature. Following the link to the PR should turn up more discussion.

@concavelenz
Copy link
Author

ic since @Kingwl contributed #40171, I shouldn't expect the core TypeScript team to take an interest in this, I suppose.

@andrewbranch
Copy link
Member

IIRC, @DanielRosenwasser is pretty enthusiastic about this flag. Personally, I think it belongs in a linter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants