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

context-related optional chaining support in paths #57

Open
lifeart opened this issue Feb 8, 2021 · 3 comments
Open

context-related optional chaining support in paths #57

lifeart opened this issue Feb 8, 2021 · 3 comments

Comments

@lifeart
Copy link
Owner

lifeart commented Feb 8, 2021

related issue: #50

for simple usage, like:

{{if this.foo.bar "a" "b"}}

{{#if this.foo.bar}}
A
{{/if}}

{{unless this.foo.bar "a" "b"}}

{{#unless this.foo.bar}}
B
{{/unless}}

{{#each this.foo.bar as |item|}}

{{/each}}

this.foo.bar path should be translated into this?.foo?.bar;

for complex usage, like:

<div {{on "click" this.foo.bar.onClick}} />

this.foo.bar.onClick should not be translated as optional chained

@boris-petrov
Copy link

A couple of comments:

  1. I'm not sure this? is sensible. When can this be undefined? Apart from template-only components, but not sure if that can't be handled in another way.
  2. Also, this.foo.bar should be treated as this.foo?.bar only if foo is nullable - e.g. public foo?: string. Otherwise it should be treated as this.foo.bar.
  3. For the on "click" case - again, the whole path should be treated the same way as point 2. It would type-check only if none of the parts are nullable (or have been check before that with an if statement) - just like in TypeScript.

@lifeart
Copy link
Owner Author

lifeart commented Feb 8, 2021

agree on 1, for 2 should we just make tail optional?

for 3 - I need to play more with if convertation, currently it's not represented as logical part, only as type (currently we can't chain type checks)

@boris-petrov
Copy link

boris-petrov commented Feb 8, 2021

Not sure what you mean by tail for number 2? You mean bar?

I think the whole thing should be mentally very simple - you have a path like this.foo.bar in HBS. We want that to type-check no matter what the path is (because that's HBS' semantics - the path will not blow up at runtime ever - but it might return undefined if any of the parts of the path returns undefined). So if foo is non-nullable (say this is { foo: { bar: string } }) then that's OK and can be translated directly to this.foo.bar. If foo is nullable (say this is { foo?: { bar: string } }) then to make this chain type-check in TypeScript we must optional-chain foo - this.foo?.bar. This would make TS happy and the whole chain would be typed as string | undefined which is exactly what the HBS type should be.

This line of thought will work for all cases we discussed in issues here and in Discord - for each, on, if, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants