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

TemplateStringsArray could contain undefined and should always have index 0 #46825

Open
LongTengDao opened this issue Nov 16, 2021 · 6 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

@LongTengDao
Copy link
Contributor

LongTengDao commented Nov 16, 2021

Bug Report

🔎 Search Terms

TemplateStringsArray undefined

🕗 Version & Regression Information

  • This is a crash
  • This changed between versions ______ and 4.4.4
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________
  • I was unable to test this on prior versions because _______

⏯ Playground Link

Playground link with relevant code 1

Playground link with relevant code 2

💻 Code

function fn (template :TemplateStringsArray) :string {
    return template.raw[0];
}
function fn (template :TemplateStringsArray) :string {
    return template[0];
}

🙁 Actual behavior

  1. the first sample throw an error (when "noUncheckedIndexedAccess": true)

  2. the second sample doesn't throw an error (when "noUncheckedIndexedAccess": false)

🙂 Expected behavior

  1. the first sample should never throw an error (because 0 index of template.raw will always be a string):
console.log``;// [ '' ]
  1. the second sample should always throw an error (because item of template could be undefined):
console.log`\unknown`;// [ undefined ]
@LongTengDao LongTengDao changed the title TemplateStringsArray could contain undefined and always have index 0 TemplateStringsArray could contain undefined and should always have index 0 Nov 16, 2021
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Nov 16, 2021

@DanielRosenwasser DanielRosenwasser added the Needs More Info The issue still hasn't been fully clarified label Nov 16, 2021
@fatcerberus
Copy link

@DanielRosenwasser
It's a bit confusingly written but the actual behavior per the OP is:

  • First function (fn) produces an error when noUncheckedIndexedAccess is Enabled.
  • Second function (fn2) doesn't produce an error when noUncheckedIndexedAccess is Disabled.

This can indeed be seen in that Playground if the aforementioned option is toggled (both functions are errors and toggling NUIA off silences them).

I'm not sure why the expected behavior is for only one case to be an error, though.

@LongTengDao
Copy link
Contributor Author

I added a more detailed explanation of the problem

@fatcerberus
Copy link

fatcerberus commented Nov 17, 2021

Yes, now it's clearer. However, both behaviors are expected:

the first sample should never throw an error (because 0 index of template.raw will always be a string)

This is an unfortunate reality when noUncheckedIndexedAccess is enabled. TS doesn't have a way to represent "non-empty array" at the type level, so you get the error. You can use a non-null assertion (!) if you're sure that the value will never be undefined at runtime.

the second sample should always throw an error (because item of template could be undefined)

Similar to above, having noUncheckedIndexedAccess disabled treats all array indexing operations as valid. That's why the flag exists in the first place. :)

In both cases, this is not a bug but (at worst) a design limitation: If your expected behavior was possible to implement, noUncheckedIndexedAccess wouldn't need to exist at all.

@RyanCavanaugh
Copy link
Member

TS doesn't have a way to represent "non-empty array" at the type level, so you get the error

This isn't quite right; the linked PR uses the tuple type [string, ...string[]] which is capable of representing non-empty string arrays.

@LongTengDao
Copy link
Contributor Author

Here are the discussion in releated PR #46826 (review):

DanielRosenwasser:

I'm not really convinced that this change really needs to happen - the | undefined change is taken care of in noUncheckedIndexedAccess, and the tuple change is at best an occasional convenience.

That said, if others want the tuple change, maybe that's worth discussing - but I'd like to see convincing use-cases when making changes to major types in lib.d.ts

I think this part is too big of a change; I would keep it as string and let people opt into this behavior with noUncheckedIndexedAccess.

LongTengDao:

According to my understanding, noUncheckedIndexedAccess is used to let the user to ensure the index is not out of length, not applicable for the use case that the item value could really be undefined.

People will forget the item of template could be undefined when noUncheckedIndexedAccess: false; and when noUncheckedIndexedAccess: true, people will also forget the item of template could be undefined because they will chronically only confirm the index is not out of length. Because this is really a niche knowledge in ECMAScript 6 that item of template could be undefined, I bet...

And this can make it consistent with TypeScript's usual behaviors:

const array :string[] = [ '' ];
array[0].length; // this warning depends on whether noUncheckedIndexedAccess is true
const array :( string | undefined )[] = [ '', , '' ];
array[0].length; // this warn even if noUncheckedIndexedAccess is false
const tuple :[ string ]= [ '' ];
tuple[0].length; // this will not warn even if noUncheckedIndexedAccess is true

Are there any downsides to this consistency? (Maybe there a risk I was overlooking)

A use case for the tuple change:

function fn ({ raw } :TemplateStringsArray, ...values :string[]) {
    let ret = raw[0];// no need to write `raw[0]!` here any more. more auto, more safe.
    for ( let index = 0; index<values.length; ++index ) {
        ret += values[index]!;
        ret += raw[++index]!;
    }
    return ret;
}

sandersn:

Let's move this discussion to an issue. The code change is not likely to be very big, but may change if the issue takes a long time to reach consensus.

@RyanCavanaugh RyanCavanaugh 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 and removed Needs More Info The issue still hasn't been fully clarified labels Feb 3, 2022
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
4 participants