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

V2 issue: inferred type disappearing #7246

Closed
1 task done
kentcdodds opened this issue Aug 24, 2023 · 29 comments · Fixed by #7605
Closed
1 task done

V2 issue: inferred type disappearing #7246

kentcdodds opened this issue Aug 24, 2023 · 29 comments · Fixed by #7605

Comments

@kentcdodds
Copy link
Member

What version of Remix are you using?

0.0.0-nightly-49e8da1-20230823

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

import * as React from "react";
import { json } from "@remix-run/node"; // types: 0.0.0-nightly-49e8da1-20230823
import { useLoaderData } from "@remix-run/react"; // types: 0.0.0-nightly-49e8da1-20230823

export async function loader() {
  return json({ payload: { name: "bob" } as Record<string, any> });
}

export default function RouteComponent() {
  const data = useLoaderData<typeof loader>();

  console.log(data.payload); // <-- Property 'payload' does not exist on type 'JsonifyObject<{}>'.ts(2339)

  return <div>stuff</div>;
}

I can't get this to reproduce in the TypeScript Playground for some reason, but here's that

Interestingly there's a difference in the type on the TS Playground and my local version:

image image

I also tried this out on a fresh project, so I'm confident it's not just my project.

Expected Behavior

I expect to be able to infer types of Record<string, any>

Actual Behavior

That property disappears.

@MichaelDeBoey MichaelDeBoey added v2 Issues related to v2 apis feat:typescript labels Aug 24, 2023
@kentcdodds
Copy link
Member Author

@kentcdodds
Copy link
Member Author

I also added a GitHub workflow. There's another TS issue in there as well (#7239), but you'll find the ts issue here: https://github.com/kentcdodds/remix-issue-7246/actions/runs/5958895147/job/16163804680#step:4:24

kentcdodds added a commit to kentcdodds/conform that referenced this issue Aug 24, 2023
The Jsonify utility used by Remix will make a `Record<string, any>` disappear in the type: remix-run/remix#7246

But considering this further, "unknown" is a safer option for this type anyway because it avoids issues with accidentally accessing the values off the submission payload without first checking them. So I do think this change should be made.
@kentcdodds
Copy link
Member Author

This is an issue with using type-fest's Jsonify which is the real culprit. Here's a TS Playground. I'm not sure whether to consider this a bug though. It feels like a bug to me... I guess they treat any as undefined for some weird reason. In any case, I think the fix will be to either not use type-fest or to have type-fest fix this.

@kentcdodds
Copy link
Member Author

This has been accepted by Sindre: sindresorhus/type-fest#667

Someone smart at TS could probably fix it pretty quick.

@kentcdodds
Copy link
Member Author

I did my best, but I couldn't get it to work: sindresorhus/type-fest#668

@brophdawg11
Copy link
Contributor

Maybe related: #6994

edmundhung pushed a commit to kentcdodds/conform that referenced this issue Aug 29, 2023
The Jsonify utility used by Remix will make a `Record<string, any>` disappear in the type: remix-run/remix#7246

But considering this further, "unknown" is a safer option for this type anyway because it avoids issues with accidentally accessing the values off the submission payload without first checking them. So I do think this change should be made.
edmundhung pushed a commit to kentcdodds/conform that referenced this issue Aug 29, 2023
The Jsonify utility used by Remix will make a `Record<string, any>` disappear in the type: remix-run/remix#7246

But considering this further, "unknown" is a safer option for this type anyway because it avoids issues with accidentally accessing the values off the submission payload without first checking them. So I do think this change should be made.
frandiox added a commit to Shopify/hydrogen that referenced this issue Aug 31, 2023
@frandiox frandiox mentioned this issue Aug 31, 2023
4 tasks
@brophdawg11 brophdawg11 removed the v2 Issues related to v2 apis label Sep 5, 2023
@rossipedia
Copy link
Contributor

rossipedia commented Sep 17, 2023

Maybe I'm missing something here, but any is the widest possible type, no? And therefore can include undefined values? In which case any type utility that intends to accurately represent the JSON serializable type representation must strip out any property with the type any. From what I understand, this is correct behavior.

If you want a type to survive the network/JSON boundary, it must not be or include undefined or else the only safe way to treat it is to strip it out, since it won't survive the JSON.stringify()/JSON.parse() marshalling.

@TheRealFlyingCoder
Copy link
Contributor

I just stumbled across this same issue in a few locations after my upgrade, so just wanted to drop a note on one thing I noticed, whether it's relevant or not.

Passing a promise through json() instead of defer() also just results in a JsonifyObject<{}>

It's kind of a seperate thing but I feel like returning a promise via json should result in a type error 👀 Such an easy thing to accidentally do

@reichhartd
Copy link
Contributor

Same for us. This error also prevents us from upgrading our projects to v2.

@smeijer
Copy link
Contributor

smeijer commented Oct 2, 2023

Here's another repl for what I believe to be the same issue.

import { json } from '@remix-run/node';
import { useLoaderData } from '@remix-run/react';
import { Schema } from '@cfworker/json-schema';

export function loader() {
    const schema = {} as Schema | null;
    return json({ schema });
}

export default function Page() {
    const { schema } = useLoaderData<typeof loader>();
    // ^ Property 'schema' does not exist on type 'JsonifyObject<{}>'. 
    return JSON.stringify(schema);
}

Playground Link: Provided

@pcattori
Copy link
Contributor

pcattori commented Oct 3, 2023

Tracked this down to this line in type-fest:

[(...arguments_: any[]) => any] extends [Type[Key]] ? never : Key;

The never is chosen for types like Schema | null, causing that entry to get filtered out.

Here's a minimal example with a suggested fix for type-fest: https://tsplay.dev/wEpQ4W

@pcattori
Copy link
Contributor

pcattori commented Oct 3, 2023

For the Record<string, any> problem, see sindresorhus/type-fest#667 (comment)

@melodyclue
Copy link

Is there a temporary workaround for this error?

@smeijer
Copy link
Contributor

smeijer commented Oct 8, 2023

The temporary issue seems to be using patch-package to revert #6642

Patch package here: https://gist.github.com/smeijer/885310fca21828272bca29097f490534

@juniorforlife
Copy link

Any updates on this issue? I can't upgrade my project to v2 with TS errors everywhere

@artecoop
Copy link

artecoop commented Oct 9, 2023

git apply --check @remix-run+server-runtime+2.0.1.patch returns the error +++ b/node_modules/@remix-run/server-runtime/dist/serialize.d.ts and even if I change server-runtime with node the file does not exists

@ZeldOcarina
Copy link

Hi same issue here, I don't quite understand how to implement this:

The temporary issue seems to be using patch-package to revert #6642

Patch package here: https://gist.github.com/smeijer/885310fca21828272bca29097f490534

@ZeldOcarina
Copy link

Ok after some fiddling I made it work, the key is using:
"postinstall": "patch-package --patch-dir ."
and placing the patch file in the root dir (or wherever is needed).
This unfortunately didn't change anything so I am just using //@ts-expect-error for now until it's fixed.

@smeijer
Copy link
Contributor

smeijer commented Oct 10, 2023

It's a patch-package patch, not a git patch.

@pcattori
Copy link
Contributor

Fixed by #7605 . You can try it out with the next nightly release (which will get published in a few hours as of the time writing this comment).

@kentcdodds
Copy link
Member Author

Hooray! Thank you!

@ZeldOcarina
Copy link

All nightly packages installed and fix is confirmed you are amazing @pcattori!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.1.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.1.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

frandiox added a commit to Shopify/hydrogen that referenced this issue Oct 26, 2023
* Update Remix versions to 2.0.0-pre.3

* Fix types in Hydrogen

* Fix type exports in Oxygen adapter

* Mock v2 flags in CLI

* Rename V2_MetaFunction type in skeleton

* Remove CSS nesting to avoid ESBuild warnings

* Remove old error and catch boundaries

* Add small wrapper for root data to fix type unknown

* ts-ignore jsonify errors
remix-run/remix#7246

* Remove vs flags from template

* Update to Remix 2.0.0

* Update skeleton types to v2

* Update hello-world types to v2

* Update demo-store types to v2

* Update examples types to v2

* Remove future v2 flags

* Minor type update

* Remove v2 flags from CLI

* Remix deprecated tsconfig in config

* Fix typecheck

* Stop injecting booleans in remix config. Fix tests

* Package lock

* Fix types

* Remove unused tests

* Cleanup

* Update package-lock

* Update Remix versions to latest nightly

* Fix optimistic-ui for Remix v2

* Update package-lock

* Remove ts-ignore comments related to JsonifyObject error

* Add missing reference to eslint package in skeleton

* Update to Remix 2.1.0

* Fix SerializeFrom in JSDoc

* Fix typedefs in JSDoc

* Use SerializeFrom in demo-store

* Changesets

* Support generics in JSDoc

* Set Remix as a non-pinned peer dependency

* Changesets

* Update .changeset/smart-ways-destroy.md

Co-authored-by: Bret Little <bret.little@shopify.com>

* Remove react dep again

* Remove template guidelines for Remix v1

* Minor fix

* Update changeset

---------

Co-authored-by: Bret Little <bret.little@shopify.com>
@AndrewOt
Copy link

I'm seeing this issue again with 2.4.0. Workaround is to do a hard cast to the type you need:

const actionResult = useActionData<typeof action>();
const errors = actionResult.formResult as MyType;

@brophdawg11
Copy link
Contributor

Could you open a new issue with a reproduction? Both reproductions in this issue are working OK for me in 2.4.0: TS Playground

@AndrewOt
Copy link

Could you open a new issue with a reproduction? Both reproductions in this issue are working OK for me in 2.4.0: TS Playground

@brophdawg11 thank you for the link, I have played a little with your example and my own use case and it seems that it just doesn't like descructuring. When I do this

const { schema } = data2;

image
It still has that JsonifyObject type in it. Then when I do this

const { key } = data2.schema;

image
It autofilled schema when I typed a . but can't recognize the key (unlike later in your example where the type of key is recognized.

So is it expected behavior for destructuring to work or am I just too much of a remix noob?

@kentcdodds
Copy link
Member Author

@AndrewOt, looks like the thing it's got a problem with destructuring is the fact that the schema could be null. Can't destructure null. You'll have to do a null-check first

@AndrewOt
Copy link

Ah. Right. It would seem I had a JS noob moment. Thanks @kentcdodds!

RickyT87411 added a commit to RickyT87411/hydrogen that referenced this issue Apr 8, 2024
* Update Remix versions to 2.0.0-pre.3

* Fix types in Hydrogen

* Fix type exports in Oxygen adapter

* Mock v2 flags in CLI

* Rename V2_MetaFunction type in skeleton

* Remove CSS nesting to avoid ESBuild warnings

* Remove old error and catch boundaries

* Add small wrapper for root data to fix type unknown

* ts-ignore jsonify errors
remix-run/remix#7246

* Remove vs flags from template

* Update to Remix 2.0.0

* Update skeleton types to v2

* Update hello-world types to v2

* Update demo-store types to v2

* Update examples types to v2

* Remove future v2 flags

* Minor type update

* Remove v2 flags from CLI

* Remix deprecated tsconfig in config

* Fix typecheck

* Stop injecting booleans in remix config. Fix tests

* Package lock

* Fix types

* Remove unused tests

* Cleanup

* Update package-lock

* Update Remix versions to latest nightly

* Fix optimistic-ui for Remix v2

* Update package-lock

* Remove ts-ignore comments related to JsonifyObject error

* Add missing reference to eslint package in skeleton

* Update to Remix 2.1.0

* Fix SerializeFrom in JSDoc

* Fix typedefs in JSDoc

* Use SerializeFrom in demo-store

* Changesets

* Support generics in JSDoc

* Set Remix as a non-pinned peer dependency

* Changesets

* Update .changeset/smart-ways-destroy.md

Co-authored-by: Bret Little <bret.little@shopify.com>

* Remove react dep again

* Remove template guidelines for Remix v1

* Minor fix

* Update changeset

---------

Co-authored-by: Bret Little <bret.little@shopify.com>
JiacaiJin pushed a commit to JiacaiJin/Shop-Hydrogen that referenced this issue May 17, 2024
* Update Remix versions to 2.0.0-pre.3

* Fix types in Hydrogen

* Fix type exports in Oxygen adapter

* Mock v2 flags in CLI

* Rename V2_MetaFunction type in skeleton

* Remove CSS nesting to avoid ESBuild warnings

* Remove old error and catch boundaries

* Add small wrapper for root data to fix type unknown

* ts-ignore jsonify errors
remix-run/remix#7246

* Remove vs flags from template

* Update to Remix 2.0.0

* Update skeleton types to v2

* Update hello-world types to v2

* Update demo-store types to v2

* Update examples types to v2

* Remove future v2 flags

* Minor type update

* Remove v2 flags from CLI

* Remix deprecated tsconfig in config

* Fix typecheck

* Stop injecting booleans in remix config. Fix tests

* Package lock

* Fix types

* Remove unused tests

* Cleanup

* Update package-lock

* Update Remix versions to latest nightly

* Fix optimistic-ui for Remix v2

* Update package-lock

* Remove ts-ignore comments related to JsonifyObject error

* Add missing reference to eslint package in skeleton

* Update to Remix 2.1.0

* Fix SerializeFrom in JSDoc

* Fix typedefs in JSDoc

* Use SerializeFrom in demo-store

* Changesets

* Support generics in JSDoc

* Set Remix as a non-pinned peer dependency

* Changesets

* Update .changeset/smart-ways-destroy.md

Co-authored-by: Bret Little <bret.little@shopify.com>

* Remove react dep again

* Remove template guidelines for Remix v1

* Minor fix

* Update changeset

---------

Co-authored-by: Bret Little <bret.little@shopify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.