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

react import should not be restricted #62939

Open
DaniGuardiola opened this issue Jun 27, 2024 · 11 comments
Open

react import should not be restricted #62939

DaniGuardiola opened this issue Jun 27, 2024 · 11 comments
Assignees
Labels
[Package] Element /packages/element [Type] Code Quality Issues or PRs that relate to code quality

Comments

@DaniGuardiola
Copy link
Member

What problem does this address?

In #54074 there was a decent amount of consensus about using (a.k.a. importing) react more directly, instead of from @wordpress/element. However, the @typescript-eslint/no-restricted-imports lint rule seems to prevent it.

What is your proposed solution?

The rule should be removed.


Note: I think it might be worth it to go the opposite direction and restrict imports from @wordpress/element. Along with a full grep+replace which shouldn't be too hard (maybe in stages?). However, for the sake of keeping a reasonable scope, let's leave that for a separate issue/conversation.

@DaniGuardiola DaniGuardiola added the [Type] Code Quality Issues or PRs that relate to code quality label Jun 27, 2024
@DaniGuardiola DaniGuardiola self-assigned this Jun 27, 2024
@Mamaduka
Copy link
Member

The same issue came up in the React compiler PR - #61788 (comment).

@DaniGuardiola
Copy link
Member Author

DaniGuardiola commented Jun 27, 2024

Thanks @Mamaduka, seems like there's even more reason to do something about this then.

Unless I hear different, I will remove the restriction as a first step, then create a different issue to propose a more active transition (which is pretty much just a grep+replace of imports), and if there aren't any big arguments against it I'll start transitioning imports in chunks until done, and they add the restriction in the opposite direction to enforce importing from "react"

@Mamaduka Mamaduka added the [Package] Element /packages/element label Jun 28, 2024
@Mamaduka
Copy link
Member

cc @WordPress/gutenberg-core

@youknowriad
Copy link
Contributor

Yeah, I'm totally ok with this change but I'm a bit concerned about losing the consistency and if we want to keep the consistency, then it's going to be a huge commit. Maybe a codemod would help.

@DaniGuardiola
Copy link
Member Author

@youknowriad I think it'll be fine to have a small transition period, and I can do my best to get PRs pumped out in quick succession. It can be done in maybe a week, as long as I can count on someone's (yours? :D) help with fast reviews.

@youknowriad
Copy link
Contributor

I think a small number of commits is actually better than a big number of PRs. That way we can add these to .git-blame-ignore-revs

@youknowriad
Copy link
Contributor

But let's gather more opinions here to see if we're not missing anything. cc @gziolo @jsnajdr

@gziolo
Copy link
Member

gziolo commented Jun 28, 2024

What benefits do we expect from these changes that justify updating the entire codebase and documentation to use React directly?

@DaniGuardiola
Copy link
Member Author

@gziolo I think the linked discussion should be relevant to your question, but to name one thing that directly impacts me: I get routinely annoyed by auto imports of react stuff resulting in eslint complaining, and having to go back and manually change it to the wrapper. I have to interrupt my flow and go and fix it.

@tyxla
Copy link
Member

tyxla commented Jul 1, 2024

I don't see much value in the @wordpress/element wrapper nowadays, it actually adds friction when a new React version comes out, and questions are coming out, like "what happens if a React drops a utility function and we need to continue to support it because of backward compatibility" or "which version of React should I use now"? By working with react APIs directly, we don't have to deal with these problems outside of the @wordpress/element package, and we leave the BC issues to the consumers to deal with. Also, the only way to no longer have to maintain these BC problems is to deprecate @wordpress/element eventually, which can happen only if the entire codebase has been migrated away from it.

To add to this, what @Mamaduka said about React Compiler not appreciating our @wordpress/element wrapper is valid, and I'm not sure they'll provide a mechanism to natively extend it to inform it that @wordpress/element is a react alias.

Also, this will help with consistency - everyone inside the codebase and outside the codebase will be able to use react imports all along. Right now you can see projects that consume both react and @wordpress/element and that's suboptimal.

It's worth noting that we'll still have to support @wordpress/element for a long time (or maybe forever) after such a migration. Also, there will be tons of developer documentation and articles that will need to be updated. With that in mind, there may be a substantial outreach effort necessary if we decide to go that way.

@Dionnie123
Copy link

Dionnie123 commented Mar 8, 2025

As a wordpress plugin developer newbie I'll just ship my own React and access it directly, I just don't have time to wait for this to happen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Element /packages/element [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

6 participants