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

Transition type needs to be imported from '@remix-run/react/dist/transition' #3866

Closed
andresBobsled opened this issue Jul 29, 2022 · 8 comments

Comments

@andresBobsled
Copy link

What version of Remix are you using?

1.6.5

Steps to Reproduce

Add Transition type to a .ts file. Only way to add the import is from '@remix-run/react/dist/transition'

Expected Behavior

Ideally we don't import dist and we can import from '@remix-run/react'

Actual Behavior

To import transition, the current line is:
import type { Transition } from '@remix-run/react/dist/transition';

@tigerabrodi
Copy link

I think Transition has to be exported here:

export type { Fetcher } from "./transition";

Then we can import it like we do with the Fetcher type.

Work around atm for stable import would be to get the return type of type useTransition ReturnType<typeof useTransition> in order to be able to use this type, or import from dist.

@kentcdodds @ryanflorence

Is there a reason not to export all types from the transition file, right now the Fetcher type is the only exported type.

@kentcdodds
Copy link
Member

Can you help me understand the use case for this?

@machour machour added needs-response We need a response from the original author about this issue/PR feat:typescript labels Jul 29, 2022
@tigerabrodi
Copy link

@kentcdodds We have a util function at Bobsled that says isSubmitting, where we pass the transition function.

I'm curious also, is there a reason not to export all types? What would be against it?

@machour machour removed the needs-response We need a response from the original author about this issue/PR label Jul 30, 2022
@kentcdodds
Copy link
Member

Just want to limit how much we expose. I think it's sensible to export this. Though @ryanflorence or @mjackson will need to approve it. Until then you can use patch-package.

@kwiat1990
Copy link

I have a similar use case as @narutosstudent.

@brophdawg11 brophdawg11 added this to v2 Aug 3, 2023
@brophdawg11 brophdawg11 moved this to Backlog in v2 Aug 3, 2023
@markdalgleish markdalgleish moved this from Backlog to In progress in v2 Aug 11, 2023
@markdalgleish
Copy link
Member

markdalgleish commented Aug 11, 2023

I think it makes sense to export this too, but as @narutosstudent mentioned, this isn't strictly an issue since you can get the type like this (note that useTransition has been deprecated in favour of useNavigation and will be removed in Remix v2):

import { useNavigation } from "@remix-run/react";

type Navigation = ReturnType<typeof useNavigation>;

@markdalgleish markdalgleish moved this from In progress to Closed in v2 Aug 11, 2023
@markdalgleish
Copy link
Member

markdalgleish commented Aug 13, 2023

The Navigation type will be exported from @remix-run/react in v2: #7136

@markdalgleish markdalgleish moved this from Closed to Merged in v2 Aug 13, 2023
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-42fe87f-20230814 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Merged
Development

No branches or pull requests

7 participants
@machour @markdalgleish @kentcdodds @kwiat1990 @tigerabrodi @andresBobsled and others