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(Jsonify): fix JsonifyTuple #638

Merged
merged 8 commits into from
Jul 9, 2023

Conversation

MichaelDeBoey
Copy link
Contributor

As pointed out by @mattcorner in remix-run/remix#6566, this would otherwise Jsonify string[] & ["Some value"] to

{
    [x: number]: "Some value";
    length: "Some value";
    toString: null;
    toLocaleString: null;
    pop: null;
    push: null;
    concat: null;
    join: null;
    reverse: null;
    shift: null;
    slice: null;
    sort: null;
    splice: null;
    ... 20 more ...;
    0: "Some value";
}

Taken from @pcattori's solution in remix-run/remix#6616

@sindresorhus
Copy link
Owner

Tests are failing

source/jsonify.d.ts Outdated Show resolved Hide resolved
source/jsonify.d.ts Outdated Show resolved Hide resolved
@MichaelDeBoey
Copy link
Contributor Author

@sindresorhus All tests are passing again

@sindresorhus sindresorhus merged commit 605b901 into sindresorhus:main Jul 9, 2023
@MichaelDeBoey MichaelDeBoey deleted the patch-1 branch July 9, 2023 12:13
@WikiRik
Copy link

WikiRik commented Jul 9, 2023

This change resulted in the following error when checking on TypeScript 4.8 for sequelize; TS2589: Type instantiation is excessively deep and possibly infinite.
See PR sequelize/sequelize#16252

@ghost
Copy link

ghost commented Jul 10, 2023

We're seeing the same:

node_modules/type-fest/source/jsonify.d.ts:22:19 - error TS2589: Type instantiation is excessively deep and possibly infinite.
22  ? FilterNonNever<[Jsonify<F>, ...JsonifyList<R>]>

@sindresorhus
Copy link
Owner

@colincollerlever What TypeScript version?

@sindresorhus
Copy link
Owner

@MichaelDeBoey @pcattori Any ideas?

@pcattori
Copy link
Contributor

Haven't looked into it, but current understanding is that its a TS 4.8 bug/limitation. Right now the options I see are:

  • a) Drop support for TS 4.7 and 4.8
  • b) Revert this PR and accept incorrect output types for input types like string{} & ["some value"]

@MichaelDeBoey
Copy link
Contributor Author

Since both are bugs, I would say:

  • revert this PR (b) and do a patch release
  • Drop support for 4.7 & 4.8 (a)
  • Add this PR again to fix the bug
  • Do a major release

sindresorhus added a commit that referenced this pull request Jul 16, 2023
sindresorhus added a commit that referenced this pull request Jul 16, 2023
@sindresorhus
Copy link
Owner

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.

4 participants