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

Incorrect inference when using the return keyword in an identity function, or when using an optional argument #49951

Closed
ShayDavidson opened this issue Jul 19, 2022 · 9 comments
Labels
Duplicate An existing issue was already created

Comments

@ShayDavidson
Copy link

ShayDavidson commented Jul 19, 2022

Bug Report

I'm running into very weird behavior when using identity functions. I'm writing a wizard system with schema (attached is a playground link with a very simplified version), and using constrained identity function to get inference.

The problem occurs in an argument of one property which is a function, when either of these occur:

  • When the returned value from the identity function is using the return keyword (rather than a single-line return wrapped with parentheses).
    OR
  • When declaring an optional argument in the identity function. The argument is declared in the type definition of the identity function, and when using Parameters<typeof myFunction> it's inferred correctly both when declaring the argument and when I not.

Both these issues are super weird to me, which means I'm either missing something very fundamental or I have found 2 rare bugs.

🔎 Search Terms

Inference, Return, Arguments, Parentheses, Identity Function.

🕗 Version & Regression Information

This reproduces in all available playground versions (tried down to 3.3.3), and also in 4.8.

⏯ Playground Link

Playground link with relevant code

💻 Code

Probably better to check the playground for code examples, but there:

TYPES DECLERATIONS:

type Schema = Record<string, unknown> // modified from original for the sake of the example, if it doesn't make sense

type StepFunction<
  TSchema extends Schema = Schema,
> = (anything: unknown) => {
  readonly schema: TSchema
  readonly toAnswers?: (keys: keyof TSchema) => unknown
}

function step<TSchema extends Schema = Schema>(
    stepVal: StepFunction<TSchema>,
  ): StepFunction<TSchema> {
    return stepVal
  }

EXAMPLES:
Notice the returned object of all functions is the same! The differences are in:

  • whether we use the return keyword or not (?!?!)
  • whether we have the argument for the step function or not. Note that if I do Parameters<typeof myStepValue> even when the argument is missing, it's inferred correctly (!)
// WORKS: `keys` is inferred based on the `schema`
// - no argument for `step` function
// - no `return` keyword
const workingExample = step(() => ({
  schema: {
    attribute: 'anything',
  },
  toAnswers: keys => {
    // RESULT: `keys` inferred successfully as `attribute`
    type Test = string extends typeof keys ? never : 'true'
    const test: Test = 'true'
    return { test }
  },
}))
// FAILS: `keys` is not inferred based on the `schema`
// - has argument for `step` function
const nonWorkingA = step(_something => ({
  schema: {
    attribute: 'anything',
  },
  toAnswers: keys => {
    // RESULT: `keys` failed to be inferred hence defaults to `string`
    type Test = string extends typeof keys ? never : 'true'
    const test: Test = 'true'
    return { test }
  },
}))
// FAILS: `keys` is not inferred based on the `schema`
// - has `return` keyword rather than a "single-return" return with parentheses
const nonWorkingB = step(() => {
  return {
    schema: {
      attribute: 'anything',
    },
    toAnswers: keys => {
      // RESULT: `keys` failed to be inferred hence defaults to `string`
      type Test = string extends typeof keys ? never : 'true'
      const test: Test = 'true'
      return { test }
    },
  }
})

🙁 Actual behavior

Nested value's function argument cannot be inferred correctly.

If I change the property (toAnswers) from a function to a simple property, there are no inference issues.

🙂 Expected behavior

Nested value function argument should be inferred correctly regardless of the return keyword or declaring the arguments.

@ShayDavidson
Copy link
Author

ShayDavidson commented Jul 20, 2022

Possible related issue for the argument case: #25092, however it was fixed in 4.7 and my issue still reproduces there.

@RyanCavanaugh
Copy link
Member

This is pretty subtle but it's indeed a consequence of #47599. When the object literal is directly in the function body expression position we can do some safer inferences that can't occur in the return version.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jul 20, 2022
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@ShayDavidson
Copy link
Author

ShayDavidson commented Jul 31, 2022

If someone ever encounters this issue, found a temp solution!

By wrapping the returned value with an identity function, both the edge cases are working, see in this playground

e.g.

step(_something => (identityOfStep({
  schema: {
    attribute: 'anything',
  },
  toAnswers: keys => {
    // RESULT: `keys` succeeds to be inferred
    type Test = string extends typeof keys ? never : 'true'
    const test: Test = 'true'
    return { test }
  },
})))

@Andarist
Copy link
Contributor

Note that nonWorkingB might get fixed if #50903 lands but noWorkingA still errors there

@Andarist
Copy link
Contributor

I think that this noWorkingA is worth a separate issue. It feels different (to me) than the other case. Ping me there and I can try to look for a fix.

@ShayDavidson
Copy link
Author

ShayDavidson commented Jan 11, 2024

Hey @Andarist! A year and a half later I've encountered notWorkingA again and now I cannot fix it with my temp solution described above.

Would you like me to open a separate issue?

@Andarist
Copy link
Contributor

This likely falls into the #47599 bucket (just like Ryan mentioned above). So it might be hard to get this fixed (that umbrella ticket isn't trivial) but maybe some carveout can be figured out for your case here, I'm not sure. I might take a look at it sometime later - and a fresh issue would definitely help me to remember about this.

@ShayDavidson
Copy link
Author

Thank you @Andarist - here's the followup #57021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants