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

Fix regression for optional JSDoc types which throws TS2463 in typescript@next #50286

Closed
voxpelli opened this issue Aug 12, 2022 · 3 comments · Fixed by #52880
Closed

Fix regression for optional JSDoc types which throws TS2463 in typescript@next #50286

voxpelli opened this issue Aug 12, 2022 · 3 comments · Fixed by #52880
Assignees
Labels
Bug A bug in TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@voxpelli
Copy link

voxpelli commented Aug 12, 2022

Bug Report

🔎 Search Terms

optional jsdoc regression

🕗 Version & Regression Information

This started occurring in nightly after #50094 was merged to fix #49869 and still happens (see these cron job type checks in pony-cause which retests against nightly three times a week).

This does not happen in 4.8 beta

  • This changed between versions 4.8-beta and nightly

⏯ Playground Link

Since the Playground is for TS code, not JS code, here's a link to a piece of code: https://github.com/voxpelli/pony-cause/blob/4009048a41d772cb29673f1c45cc55e9717e523f/lib/error-with-cause.js#L5-L9

And more in this thread: voxpelli/pony-cause#45

💻 Code

   /** 
    * @param {{ cause?: string }} [options] 
    */ 
   function foo ({ cause } = {}) { 

🙁 Actual behavior

Explicitly specifying the options as optional ([options] ) causes this error as it is already implicitly marked as optional by having been set a default value ({ cause } = {}):

error TS2463: A binding pattern parameter cannot be optional in an implementation signature.

🙂 Expected behavior

One of:

  1. Preferably that TypeScript keep accepting arguments that's been explicitly marked as optional, even if that is redundant
  2. At least change the error message to something that makes more sense in a JSDoc context than "A binding pattern parameter cannot be optional in an implementation signature."

See more in this thread: voxpelli/pony-cause#45 and in #50094 (comment), where @sandersn said that he is in favor the current new behavior.

@voxpelli
Copy link
Author

Seems like this is actually affecting TS 4.8.2

voxpelli added a commit to voxpelli/node-pg-pubsub that referenced this issue Aug 26, 2022
jaydenseric added a commit to jaydenseric/next-graphql-react that referenced this issue Aug 29, 2022
Includes a workaround for a TypeScript v4.8.2 bug: microsoft/TypeScript#50286 .
jaydenseric added a commit to jaydenseric/graphql-react that referenced this issue Aug 29, 2022
Includes a workaround for a TypeScript v4.8.2 bug: microsoft/TypeScript#50286 .
@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this Domain: JSDoc Relates to JSDoc parsing and type generation and removed Help Wanted You can do this labels Aug 30, 2022
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.9.0 milestone Aug 30, 2022
babichjacob added a commit to babichjacob/svelte-localstorage that referenced this issue Sep 5, 2022
jespertheend added a commit to jespertheend/deno-tsc-helper that referenced this issue Sep 7, 2022
saschanaz added a commit to w3c/webidl2.js that referenced this issue Sep 10, 2022
Pinning TypeScript version because of microsoft/TypeScript#50286.
@mohd-akram
Copy link

Any update on this?

@turadg
Copy link

turadg commented Sep 25, 2022

Any update on this?

Here is a TS playground link showing it's still a problem in 4.9.0-dev.20220921 (Nightly). Also 4.8.2, and not 4.7.4.

However it now has an assignee and is part of the TypeScript 4.9.0 milestone so I think you can expect it to be fixed in a future nightly.

boneskull added a commit to headspinio/appium-tizen-tv-driver that referenced this issue Sep 28, 2022
the only time when we should specifically force a new token is when `appium:resetRcToken` is `true`.

also, immediately after instantiation, unless a token has been provided to `TizenRemote` (or via the env), the `hasToken()` method may resolve `true`, but it will not actually _set_ the token in memory from the cache due to it being a naughty side-effect.  unless `getToken()` was explicitly called, the token would not be set in-memory from the cache before the connection attempt.  the logic has changed so that, at connection time, we attempt to load the token from cache if one exists (and cache persistence is enabled).  if one still doesn't exist after connection, then it will ask for a new one.

also fixes some typescript problems which may actually be a bug in typescript. microsoft/TypeScript#50286

adds a (slow) e2e test suite for checking persistence works
boneskull added a commit to headspinio/appium-tizen-tv-driver that referenced this issue Sep 28, 2022
the only time when we should specifically force a new token is when `appium:resetRcToken` is `true`.

also, immediately after instantiation, unless a token has been provided to `TizenRemote` (or via the env), the `hasToken()` method may resolve `true`, but it will not actually _set_ the token in memory from the cache due to it being a naughty side-effect.  unless `getToken()` was explicitly called, the token would not be set in-memory from the cache before the connection attempt.  the logic has changed so that, at connection time, we attempt to load the token from cache if one exists (and cache persistence is enabled).  if one still doesn't exist after connection, then it will ask for a new one.

also fixes some typescript problems which may actually be a bug in typescript. microsoft/TypeScript#50286

adds a (slow) e2e test suite for checking persistence works
turadg added a commit to endojs/endo that referenced this issue Oct 26, 2022
turadg added a commit to endojs/endo that referenced this issue Oct 26, 2022
turadg added a commit to endojs/endo that referenced this issue Oct 28, 2022
jaydenseric added a commit to jaydenseric/find-unused-exports that referenced this issue Nov 1, 2022
Avoid function `findUnusedExports` parameter destructuring to workaround a recently introduced TypeScript bug:

microsoft/TypeScript#50286
voxpelli added a commit to voxpelli/semver-set that referenced this issue Nov 11, 2022
voxpelli added a commit to voxpelli/node-promised-retry that referenced this issue Nov 11, 2022
turadg added a commit to Agoric/agoric-sdk that referenced this issue Nov 16, 2022
turadg added a commit to Agoric/agoric-sdk that referenced this issue Nov 16, 2022
turadg added a commit to Agoric/agoric-sdk that referenced this issue Nov 17, 2022
turadg added a commit to Agoric/agoric-sdk that referenced this issue Nov 18, 2022
@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Feb 1, 2023
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Feb 20, 2023
denisp22 pushed a commit to denisp22/graphql-react that referenced this issue Jul 27, 2023
Includes a workaround for a TypeScript v4.8.2 bug: microsoft/TypeScript#50286 .
jaydenseric added a commit to jaydenseric/find-unused-exports that referenced this issue Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JSDoc Relates to JSDoc parsing and type generation Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
6 participants