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

Support optional chaining #4303

Closed
vjpr opened this issue Jul 3, 2017 · 55 comments
Closed

Support optional chaining #4303

vjpr opened this issue Jul 3, 2017 · 55 comments

Comments

@vjpr
Copy link

vjpr commented Jul 3, 2017

When could we expect optional chaining support.

It will be released soon with babel@7.

babel/babylon#545

@7ynk3r
Copy link

7ynk3r commented Sep 19, 2017

this is super important to make the code more readable. i'd love to have this.

@eshikerya
Copy link

this feature is really important for project i am working for. waiting for implementation!

@suparngp
Copy link

This is really important for dealing with JSON responses without having to write a plethora of null checks. +1

@alangpierce
Copy link
Contributor

Note that optional chaining is still a stage 1 proposal and has gone through a number of changes since the Babel plugin was implemented. It's very likely that there will be changes to both the syntax and the behavior before the final JS feature is released.

See various other discussions:
https://github.com/tc39/proposal-optional-chaining
https://github.com/rwaldron/tc39-notes
microsoft/TypeScript#16

@nodkz
Copy link
Contributor

nodkz commented Jan 3, 2018

Can be a simple case of optional chaining (optional static property access) implemented today under option's flag esproposal or experemental?

I say about simple property reading from nested objects a?.b?.c, which will be an amazing improvement in the code for React components. Eg when reading graphql responses.

And leave dynamic and function/method calls uncovered while it in stage-1.

cc @calebmer @mroch

@zanettin
Copy link

which will be an amazing improvement in the code for React components. Eg when reading graphql responses.

would love to see that! we could remove tons of code and checks if flow also supports this syntax. thanks @nodkz and @alangpierce for having an eye on that.

@cloudkite
Copy link
Contributor

looks like this will be in the next version 🎉763ffb5

@niieani
Copy link

niieani commented Mar 5, 2018

@cloudkite looks like only parsing will be supported. Type support isn't landing, as the feature has been marked as experimental.

@fishythefish fishythefish self-assigned this Mar 6, 2018
@fishythefish
Copy link

I've added (most) type support for optional chaining in 0.71.0 (gated behind the esproposal.optional_chaining flag). The exception is that you'll get a Flow error if you try to call a member expression where the call and/or member is part of an optional chain. Once I get optional chaining to play nice with our implementation of methods, I'll remove this restriction.

@houshuang
Copy link

This is amazing, I've been waiting for this.

In the following situation it complains about dash.exampleLogs[x] on line 319 because dash.exampleLogs might be undefined, however given that this code will only run if line 318 is true, shouldn't Flow realize that this cannot be undefined in this context?

Cannot get dash.exampleLogs[x] because an indexer property is missing in undefined [1].

     frog/imports/ui/Preview/ShowDashExample.js
     316               const dash = aTdashs[this.state.example];
     317               let data;
     318               if (dash?.exampleLogs?.[x].type === 'state') {
     319                 data = dash.exampleLogs[x].state;
     320               } else if (dash.prepareDataForDisplay) {
     321                 data = dash.prepareDataForDisplay(
     322                   dash.initData,

     frog-utils/dist/types.js.flow
 [1] 157     | {
     158         type: 'logs',
     159         title: string,
     160         path: string,
     161         activityMerge?: Object,
     162         instances?: number
     163       }
     164     | { title: string, type: 'state', activityMerge?: Object, state: any }
     165   )[]

@fishythefish
Copy link

@houshuang We could potentially do that, but Flow doesn't currently perform that refinement.

@mrkev
Copy link
Contributor

mrkev commented May 7, 2018

@houshuang feel free to create a separate issue to track and discuss that

@murrayju
Copy link

@houshuang @fishythefish @mrkev was a separate issue ever created? I too am finding it hard to make use of optional chaining when flow complains that I haven't checked for null (when I have).

@mrkev
Copy link
Contributor

mrkev commented Jul 19, 2018

I haven't seen one.

@Hypnosphi
Copy link
Contributor

#6613

@0rvar
Copy link

0rvar commented Aug 13, 2018

Babel 7 RC has been released - it would be nice to have this and null coalescing ready when Babel 7 is fully released

@0rvar
Copy link

0rvar commented Aug 22, 2018

Babel 7 is being released this week:
https://twitter.com/left_pad/status/1031700991186489344

@bsmith-cycorp
Copy link

Any progress on this? Babel 7 is now fully released: https://babeljs.io/blog/2018/08/27/7.0.0

@vicapow
Copy link
Contributor

vicapow commented Aug 29, 2018

I can only speculate as I'm not a core contributor to Flow but Just because babel supports a feature, doesn't mean Flow will or wont support it. optional chaining happens to be partially supported in Flow already but function calls are not yet supported. Maybe someone from the Flow core team could comment on if adding full support is planned? Or is this a feature the core team would be willing to merge but not prioritized?

@eshikerya
Copy link

function calls not supported and typing also. the optional property become ?any type(afaik)

@vicapow
Copy link
Contributor

vicapow commented Aug 29, 2018

@eshikerya If typing is broken, that would be an issue but as far as I've seen, typing when using optional chaining is working as expected. Here's an example. Could you provide an example that demonstrates the issue you're seeing?

function func(prop1 : ? { prop2?: string }) {
  return prop1?.prop2;
}

// no error
(func({bar: { bar : 'wow' }}): ?string);
// errors as expected
(func({bar: { bar : 'wow' }}): ?number);

@vicapow
Copy link
Contributor

vicapow commented Aug 29, 2018

cc @samwgoldman or @avikchaudhuri

@bsmith-cycorp
Copy link

@vicapow it seems that basic property access work, but methods do not:
solo_react_js_ ___svn_cyc_doc_app_src_cyc-ui

@eshikerya
Copy link

eshikerya commented Aug 29, 2018

with flow@0.78 it works as expected

type Dictionary<K, V> = { [id: K]: V };

type enmWorkspaceUserRoleType =
  | "Nobody"
  | "Resource"
  | "Spectator"
  | "Participant"
  | "Leader"
  | "Owner";

type WorkspaceUserInfoType = {
  id: ID,
  name: string,
  role: enmWorkspaceUserRoleType
};
type WorkspaceInfoType = {
  id: ID,
  name: string,
  workspaceUser?: WorkspaceUserInfoType
};

const workspaces: Dictionary<string, WorkspaceInfoType> = {
  testId: {
    id: "testId",
    name: "Test Workspace",
    workspaceUser: {
      id: "userId",
      name: "Test User",
      role: "Owner"
    }
  }
};

console.log(workspaces.testId?.workspaceUser?.role);

expected error will be reported in case:

console.log(workspaces.testId?.workspaceUser.role);

Cannot get `workspaces.testId?.workspaceUser.role` because property `role` is missing in undefined [1]. [1]

@fishythefish
Copy link

Assuming the proposal remains unchanged, the plan is to eventually add support for calls in optional chains. However, we're waiting for the proposal to progress beyond stage 1, since it's unclear whether optional calls will make it into the final proposal and, if so, what semantics they will have; see tc39/proposal-optional-chaining#59.

@gnmerritt
Copy link

Are we at the point where an implementation PR would be accepted? I'd be happy to take a shot at getting it working if that's the only hurdle left.

@mrkev
Copy link
Contributor

mrkev commented Apr 1, 2019

IIRC this was waiting for the proposal to go through, though I might be wrong. Someone in the team might know better. @dsainati1

@dsainati1
Copy link
Contributor

I'm not an expert on this unfortunately so I don't have any new information.

@mrkev
Copy link
Contributor

mrkev commented Apr 1, 2019

@dsainati1 who would know if this is something that can be added?

@dsainati1
Copy link
Contributor

Not sure honestly. Maybe @nmote?

@elquimista
Copy link

Optional chaining has been brought to stage-2 yesterday. Can we move forward now, Flow?

tc39/proposals@effb785
tc39/proposal-optional-chaining#83 (comment)

@kenrick95
Copy link

Update: Optional chaining has moved to Stage 3

tc39/proposals@25f8d6a
tc39/proposal-optional-chaining@73cf985

@goodmind
Copy link
Contributor

@kenrick95 that was fast

@goodmind
Copy link
Contributor

@mroch is it possible to support optional calls?

@gajus
Copy link

gajus commented Aug 28, 2019

It is now supported by v8.

https://v8.dev/features/optional-chaining

@gajus
Copy link

gajus commented Aug 28, 2019

Sorry – typo. It was supposed to say:

It is now supported by v8.

Edited.

@nmote
Copy link
Contributor

nmote commented Aug 29, 2019

At this point it's just a matter of prioritization. The worry with implementing early-stage proposals is that we will implement something that does not match the final specification of the feature, leading to a painful non-backwards-compatible change down the road. At this point, I wouldn't worry about that with optional chaining.

@lxe
Copy link

lxe commented Oct 2, 2019

Is it possible to change Flow does not yet support method or property calls in optional chains. error into a warning?

@ovidb
Copy link

ovidb commented Nov 19, 2019

Any update on the priority of proper support for optional chaining?

We (or at least I) think that flow is really great and we love using it, but sometimes you can't help but ask yourself if it's time to move to TypeScript, which now support optional chaining and null coalescing.

@TrySound
Copy link
Contributor

@ovidb Flow supports both for a long time. Optional calls support was introduced in the latest release.

@ovidb
Copy link

ovidb commented Nov 19, 2019

@ovidb Flow supports both for a long time. Optional calls support was introduced in the latest release.

@TrySound I apologise for not checking the release notes, but I am on the latest version and the reason I thought it's still not supported is because whenever you use optional chaining inside an if statement, flow doesn't seem to understand that that maybe value has been checked for. Is this a bug or a known fact?

https://flow.org/try/#0C4TwDgpgBA8mwEsD2A7AhgGygXigbwCgBINAfgC58CooiiAjSgZ2ACcEUBzAGmIF8CAggDMArigDGiVFGAQWcaegwAKJPGTLKizZgCUVIgmFQ1G1JgB0ZS-QOEajx+qVW0ty8CQBlNh04qegDc1I4CAk5OoZEE0U5AA

On the bright side, thanks for letting us know that calls support is now working. Thats amazing news!!!

@TrySound
Copy link
Contributor

Yes, currently refinement is not preserved. So you need to cache optional chaining result in variable or use null check in if statement.

@TrySound
Copy link
Contributor

#6613

@gnprice
Copy link
Contributor

gnprice commented Aug 4, 2020

FTR in case it's helpful for anyone else, it looks like v0.112.0 was the release that introduced support for method calls in optional chains.

I determined that using this handy demo from a comment above:

This works: https://flow.org/try/#0C4TwDgpgBA8mwEsD2A7AhgGygXigbwCgoo0B+ALnyKgEgAjSgZ2ACcEUBzAGmoF8D+BAGYBXFAGNEqKMAjM4U9BgAUSeMiWUFGzAEoqxNYswA6MibongSAMqt2HZboDc1BEKir1qU2igBCXBQRDAx9QmIaIx0MMwsrW3tOJ1difl4gA

and a quick binary search. It's also mentioned in the release notes under v0.112.0, though not very definitively.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 18, 2020
Prompted by discussion on zulip#4101 [1].

Flow started supporting method calls in optional chains in
v0.112.0 [2], but ESLint isn't yet on board [3]; it gives a false
positive for `no-unused-expressions`.

A comment on the ESLint issue [4] suggests we could be more
systematic by using an ESLint plugin from Babel, instead of one-off
suppressions of `no-unused-expressions` like this one. That plugin
moved from (on NPM) `babel-eslint-plugin` to `@babel/eslint-plugin`.
We can't use the new one until we're on ESLint 7 (see zulip#4254), but we
could probably use the old one.

[1] zulip#4101 (comment)
[2] facebook/flow#4303
[3] eslint/eslint#11045
[4] eslint/eslint#11045 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 19, 2020
Prompted by discussion on zulip#4101 [1].

Flow started supporting method calls in optional chains in
v0.112.0 [2], but ESLint isn't yet on board [3]; it gives a false
positive for `no-unused-expressions`.

A comment on the ESLint issue [4] suggests we could be more
systematic by using an ESLint plugin from Babel, instead of one-off
suppressions of `no-unused-expressions` like this one. That plugin
moved from (on NPM) `babel-eslint-plugin` to `@babel/eslint-plugin`.
We can't use the new one until we're on ESLint 7 (see zulip#4254), but we
could probably use the old one.

[1] zulip#4101 (comment)
[2] facebook/flow#4303
[3] eslint/eslint#11045
[4] eslint/eslint#11045 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 21, 2020
Prompted by discussion on zulip#4101 [1].

Flow started supporting method calls in optional chains in
v0.112.0 [2], but ESLint isn't yet on board [3]; it gives a false
positive for `no-unused-expressions`.

A comment on the ESLint issue [4] suggests we could be more
systematic by using an ESLint plugin from Babel, instead of one-off
suppressions of `no-unused-expressions` like this one. That plugin
moved from (on NPM) `babel-eslint-plugin` to `@babel/eslint-plugin`.
We can't use the new one until we're on ESLint 7 (see zulip#4254), but we
could probably use the old one.

[1] zulip#4101 (comment)
[2] facebook/flow#4303
[3] eslint/eslint#11045
[4] eslint/eslint#11045 (comment)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 21, 2020
Prompted by discussion on zulip#4101 [1].

Flow started supporting method calls in optional chains in
v0.112.0 [2], but ESLint isn't yet on board [3]; it gives a false
positive for `no-unused-expressions`.

A comment on the ESLint issue [4] suggests we could be more
systematic by using an ESLint plugin from Babel, instead of one-off
suppressions of `no-unused-expressions` like this one. That plugin
moved from (on NPM) `babel-eslint-plugin` to `@babel/eslint-plugin`.
We can't use the new one until we're on ESLint 7 (see zulip#4254), but we
could probably use the old one.

[1] zulip#4101 (comment)
[2] facebook/flow#4303
[3] eslint/eslint#11045
[4] eslint/eslint#11045 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests