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

Added failing test for strange Promise.reject issue #2970

Closed

Conversation

mattpocock
Copy link
Contributor

No description provided.

@mattpocock mattpocock requested a review from Andarist January 26, 2022 16:24
@changeset-bot
Copy link

changeset-bot bot commented Jan 26, 2022

⚠️ No Changeset found

Latest commit: a0f3c61

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a0f3c61:

Sandbox Source
XState Example Template Configuration
XState React Template Configuration

@ghost
Copy link

ghost commented Jan 26, 2022

CodeSee Review Map:

Review these changes using an interactive CodeSee Map

Review in an interactive map

View more CodeSee Maps

Legend

CodeSee Map Legend

@Andarist
Copy link
Member

I think this is a "wont fix" for now. It looks like a bug. I was able to create a minimal repro case for this problem.

I think that this is what happens:

  1. TS grabs the function type from this service property
  2. it gets its return type and provides the contextual type (the provided string), this expands the union from like Union<T> to Union<string>
  3. it recognized the function as being async so it wraps that return type in a Promise, so the type becomes Promise<Union<string>>
  4. it checks if the type is a union and if it is then it would try to narrow it down, this is probably what happens for my test4, However, at this time this type is not a union so this stage is being skipped

I think that, potentially, this could even be an easy fix in TS itself -> just move the narrowing logic before wrapping the type into a Promise type. However, promises have this "auto-flattening" behavior that is recursive and maybe this is why this (unconfirmed) order of operation has been chosen. Although, it feels that this wouldn't change that much as the final "flattening"/unwrapping should happen at the end - hard to tell without examining more situations and edge cases.

Base automatically changed from andarist/typegen-types to main January 27, 2022 12:39
@Andarist
Copy link
Member

Andarist commented Sep 13, 2023

This is an upstream issue in TS (see here) and the fix for it awaits review here. It's not actionable on our side here.

@Andarist Andarist closed this Sep 13, 2023
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

Successfully merging this pull request may close these issues.

2 participants