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

Warn exports instead of components when using only components #33

Open
skovhus opened this issue Dec 4, 2023 · 3 comments
Open

Warn exports instead of components when using only components #33

skovhus opened this issue Dec 4, 2023 · 3 comments

Comments

@skovhus
Copy link

skovhus commented Dec 4, 2023

It seems like object export isn't correctly handled. In the following simplified example, NameInput has a warning, but it should actually be SomeInternalInput and SomeInternalForm OR TitleSection that had an error.

const SomeInternalInput = () => <div><NameInput /></div>
const SomeInternalForm = () => <div />

// Shows a warning, even though it isn't exported
const NameInput = styled(Input)`
  font-weight: 500;
`;

export const TitleSection = {
  Input: SomeInternalInput,
  Form: SomeInternalForm,
};
@ArnaudBarre
Copy link
Owner

I can try to better report errors, but this case is justified and the error is always more for the file than for a specific export.

This file can't be correctly Fast Refresh sadly (this technically could be should have support from the runtime to check that the object only export components et exactly the same keys than in the previous update, which not a one line simple update)

@skovhus
Copy link
Author

skovhus commented Dec 4, 2023

Thanks. Yeah I understand that it isn't supported by fast refresh, but it would be great to improve the warning placement to the actual variables that are exported.

@ArnaudBarre ArnaudBarre changed the title Support export object pattern Warn exports instead of components when using only components Dec 4, 2023
@ArnaudBarre
Copy link
Owner

So actually I've look at it and it correctly warns on all components that could loose state on Fast Refresh.
The action is too split the components from the export, sometimes it would mean moving the component to another file because it makes sense to export an object containing a component in that file, sometimes this would mean moving the export to another file because it's mostly an helper.
I think the current behaviour is ok but I'm open to changing the warning message to something else that would make it more clear when on the export side

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

No branches or pull requests

2 participants