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

Use a consistent ordering when writing union types #17944

Open
ghost opened this issue Aug 21, 2017 · 5 comments
Open

Use a consistent ordering when writing union types #17944

ghost opened this issue Aug 21, 2017 · 5 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: Type Display Bugs relating to showing types in Quick Info/Tooltips, Signature Help, or Completion Info Suggestion An idea for TypeScript

Comments

@ghost
Copy link

ghost commented Aug 21, 2017

In DefinitelyTyped, $ExpectType assertions depend on a type having a consistent string represenation across TypeScript versions.
It looks like the ordering of union types is dependent on some implementation details that change between versions. When we output unions as strings, we should sort them in some consistent way first instead of just using whatever order we happened to use internally.

I would suggest this ordering:

  • numeric literals, low to high
  • string literals, low to high (by <)
  • named types (including type aliases, enum, class and interface names), by name
  • function literals: by length (() => void before (x: number) => void), then by parameter name, then by parameter type
  • type literals: Sorted smallest to largest ({ x: number } before { x: number, y: number }); and type literals of the same size should be alphabetically sorted by property ({ a: number }) before { b: number }) or by the sorting of values ({ s: "a" } before { s: "b" })

Of course, another solution would be to try to handle this in $ExpectType by parsing out unions and allowing it if any sorting is valid. But it is strange in TS to see a string literal union displayed in a seemingly random order.

@mhegazy mhegazy added the Suggestion An idea for TypeScript label Aug 21, 2017
@DanielRosenwasser
Copy link
Member

While I understand the I understand the frustration, I don't think this is a good idea from a user perspective. Why would you want the user to have to memorize the rules for union type ordering?

In my opinion, you can go one of two paths:

  1. The tool should be user-friendly enough and perform its own sorting.
  2. Introduce a snapshot-based approach where users can accept baselines.

Additionally, I'm not sure how bullish I am on the idea of committing to a specific ordering.

@ghost
Copy link
Author

ghost commented Aug 22, 2017

The goal isn't for users to memorize the ordering, the goal is for our output to be consistent across versions to that we can run the same tests against multiple versions. A baseline approach would be a bad idea as currently the ordering could change in nightly builds, which could cause hundreds of breaks in DT tests if we do something that changes the order of type creation.
I've created microsoft/dtslint#61 to clean up the test failures for now, could you review?

@RyanCavanaugh
Copy link
Member

I'm with Andy on this one - we get "bug" reports because people don't understand why the order in quick info isn't the same order they wrote (e.g. you can write A | B and you'll sometimes see B | A). Being able to explain why that order is, rather than "It's arbitrary, so ... ??" would be preferable.

There's also the problem of devs on the same time with slightly different compiler versions emitting .d.ts files and constantly generating false diffs - because we don't guarantee this ordering at all, even patch versions could cause this behavior.

@DanielRosenwasser DanielRosenwasser added the Domain: Type Display Bugs relating to showing types in Quick Info/Tooltips, Signature Help, or Completion Info label May 24, 2018
@RyanCavanaugh RyanCavanaugh added the Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature label Aug 15, 2018
@Blasz
Copy link
Contributor

Blasz commented Sep 2, 2019

There's also the problem of devs on the same time with slightly different compiler versions emitting .d.ts files and constantly generating false diffs - because we don't guarantee this ordering at all, even patch versions could cause this behavior.

I'm running into this issue right now. We have a process that diffs our d.ts files to test against any changes as part of build refactors. This is generating false positives because of the union order not being idempotent, regardless of compiler versions.

Is there any update on this? Based on this comment #13298 (comment) it seems like this isn't possible to implement?

@AnyhowStep
Copy link
Contributor

#32224

drarmstr added a commit to drarmstr/Recoil that referenced this issue Mar 30, 2022
Summary:
It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: b63c6165393727970468979d5a330f8d13a0ffe5
drarmstr added a commit to drarmstr/Recoil that referenced this issue Mar 31, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: 90bc30cb3bbd4017746d0ac96d9c5277de13a1f9
drarmstr added a commit to drarmstr/Recoil that referenced this issue Mar 31, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: da9d59ba1995ad85dd5bea1c3ae87eb8728df2a4
drarmstr added a commit to drarmstr/Recoil that referenced this issue Mar 31, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: https://www.internalfb.com/diff/D35266493?entry_point=27

fbshipit-source-id: 0b9a7d4f73455ec92ef9a9321c7dc7797c1b5c1f
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 1, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: a065bf0c91d4e96c2b56ded885b979312df89b2e
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 2, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: 76f949ed92cad2e1fd0b21482bfdace11774a020
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 2, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: afaaa76192361d0537bd343fefcea0e2ff690506
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 2, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: 5a0e602657998d988bfa1b88d57e456561d6bc2e
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 5, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: 6622154e1fa921a67f0d87363abf844cefa396f0
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 5, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: a7430abeabb61bfac677d0a04a015e4504905a92
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 5, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: aad7a881ca5f98885cfba1b41f06b098f0bb3168
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 6, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: 6fbbe84ebf39794426f6e914145d49ce4fbb0150
drarmstr added a commit to drarmstr/Recoil that referenced this issue Apr 6, 2022
Summary:
Pull Request resolved: facebookexperimental#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Differential Revision: D35266493

fbshipit-source-id: f5ff27c31b98a5917cd0102ed05db23cd4fd4b5a
facebook-github-bot pushed a commit to facebookexperimental/Recoil that referenced this issue Apr 6, 2022
Summary:
Pull Request resolved: #1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Reviewed By: habond

Differential Revision: D35266493

fbshipit-source-id: a5984e16c5dac12896d90f96e0e7808ab3b69948
AlexGuz23 pushed a commit to AlexGuz23/Recoil that referenced this issue Nov 3, 2022
Summary:
Pull Request resolved: facebookexperimental/Recoil#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Reviewed By: habond

Differential Revision: D35266493

fbshipit-source-id: a5984e16c5dac12896d90f96e0e7808ab3b69948
snipershooter0701 pushed a commit to snipershooter0701/Recoil that referenced this issue Mar 5, 2023
Summary:
Pull Request resolved: facebookexperimental/Recoil#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Reviewed By: habond

Differential Revision: D35266493

fbshipit-source-id: a5984e16c5dac12896d90f96e0e7808ab3b69948
eagle2722 added a commit to eagle2722/Recoil that referenced this issue Sep 21, 2024
Summary:
Pull Request resolved: facebookexperimental/Recoil#1704

It appears that TypeScript can produce inconsistent ordering for union types across versions while `$ExpectType` requires a consistent ordering.  (microsoft/TypeScript#17944) This can cause our TypeScript tests to break when run across multiple TypeScript versions.

Update the tests to be more robust to work across versions.

Reviewed By: habond

Differential Revision: D35266493

fbshipit-source-id: a5984e16c5dac12896d90f96e0e7808ab3b69948
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: Type Display Bugs relating to showing types in Quick Info/Tooltips, Signature Help, or Completion Info Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants