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

useLoaderData types broken if unstable_data is conditionally used #9826

Closed
michaelgmcd opened this issue Aug 5, 2024 · 6 comments · Fixed by #9999
Closed

useLoaderData types broken if unstable_data is conditionally used #9826

michaelgmcd opened this issue Aug 5, 2024 · 6 comments · Fixed by #9999
Labels
awaiting release This issue has been fixed and will be released soon bug:unverified feat:single-fetch feat:typescript

Comments

@michaelgmcd
Copy link
Contributor

Reproduction

With the following loader:

export const loader = unstable_defineLoader(async ({ request }) => {
  const isBot = isbot(request.headers.get('user-agent'));

  // Prevents making a session for every bot request
  if (isBot) {
    return { notification: null };
  }

  const session = await sessionStorage.getSession(request.headers.get('Cookie'));
  const notification = session.get('notification') ?? null;

  return unstable_data({ notification }, { 'Set-Cookie': await sessionStorage.commitSession(session) });
});

const { notification } = useLoaderData<typeof loader>(); returns a type issue. The issue exists because of the conditional use of unstable_data. If I remove the "isBot" block, the type issue is gone.

Screenshot 2024-08-04 at 9 01 46 PM

System Info

System:
    OS: macOS 14.5
    CPU: (10) arm64 Apple M2 Pro
    Memory: 2.85 GB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.16.0 - /opt/n/bin/node
    npm: 10.8.1 - /opt/n/bin/npm
    pnpm: 9.4.0 - /opt/n/bin/pnpm
    Watchman: 2024.06.17.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 127.0.6533.89
    Safari: 17.5

Used Package Manager

pnpm

Expected Behavior

No type issue. In the use case above, notification would be NotificationType | null

Actual Behavior

Type issue attached above.

@michaelgmcd
Copy link
Contributor Author

The following works for us. Happy to make a PR. "Unwrap" naming could be improved.

type Serializable =
    | undefined
    | null
    | boolean
    | string
    | symbol
    | number
    | Serializable[]
    | { [key: PropertyKey]: Serializable }
    | bigint
    | Date
    | URL
    | RegExp
    | Error
    | Map<Serializable, Serializable>
    | Set<Serializable>
    | Promise<Serializable>;

  type DataFunctionReturnValue =
    | Serializable
    | DataWithResponseInit<Serializable>
    | TypedDeferredData<Record<string, unknown>>
    | TypedResponse<Record<string, unknown>>;

  export type Unwrap<T extends DataFunctionReturnValue> =
    T extends TypedDeferredData<infer D>
      ? D
      : T extends TypedResponse<Record<string, unknown>>
        ? SerializeFrom<T>
        : T extends DataWithResponseInit<infer D>
          ? D
          : T;

  export type Serialize<T extends Loader | Action> =
    Awaited<ReturnType<T>> extends DataFunctionReturnValue
      ? Unwrap<Awaited<ReturnType<T>>>
      : Awaited<ReturnType<T>>;

@brophdawg11
Copy link
Contributor

This is resolved by #9999 and will be included in the next release 👍

@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Sep 18, 2024
Copy link
Contributor

🤖 Hello there,

We just published version 2.12.1-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!

Copy link
Contributor

🤖 Hello there,

We just published version 2.12.1 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!

@zbinlin
Copy link

zbinlin commented Nov 19, 2024

@brophdawg11 I have same issue in remix v2.14.0

import { type LoaderFunctionArgs, data } from "@remix-run/node";
import { useLoaderData } from "@remix-run/react";

export const loader = async () => {
	return data({
		foo: "bar",
	}, {
		headers: {
			"Set-Cookie": "Foo=Bar",
		},
	});
};

export default function Index() {
	const loaderData = useLoaderData<typeof loader>();
	// ^^^ The `loaderData` type is JsonifyObject<DataWithResponseInit<{ foo: string }>>
	// Expect JsonifyObject<{ foo: string }>
}

@goodeats
Copy link

goodeats commented Nov 21, 2024

@brophdawg11 I have same issue in remix v2.14.0

import { type LoaderFunctionArgs, data } from "@remix-run/node";
import { useLoaderData } from "@remix-run/react";

export const loader = async () => {
	return data({
		foo: "bar",
	}, {
		headers: {
			"Set-Cookie": "Foo=Bar",
		},
	});
};

export default function Index() {
	const loaderData = useLoaderData<typeof loader>();
	// ^^^ The `loaderData` type is JsonifyObject<DataWithResponseInit<{ foo: string }>>
	// Expect JsonifyObject<{ foo: string }>
}

I was running into this too and what solved it for me was combining these two parts from Essential Typescript for React:

  • Use ReturnType and Awaited
  • Use Omit to remove props when you override them.

So something like:

type LoaderData = Omit<Awaited<ReturnType<typeof loader>>, 'foo'> & {
	foo: string
}

const loaderData = useLoaderData<LoaderData>();

I'm sure this will be patched at some point, but this seems to work in the meantime.

Edit:

I forgot to declare the module as well in vite.config.ts

declare module '@remix-run/node' {
	interface Future {
		v3_singleFetch: true
	}
}

Now const loaderData = useLoaderData<typeof loader>(); is working as expected for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting release This issue has been fixed and will be released soon bug:unverified feat:single-fetch feat:typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants