search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
9.98k stars 4.01k forks source link

`react` import should not be restricted #62939

Open DaniGuardiola opened 3 days ago

DaniGuardiola commented 3 days ago

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.

Mamaduka commented 3 days ago

The same issue came up in the React compiler PR - https://github.com/WordPress/gutenberg/pull/61788#issuecomment-2142159849.

DaniGuardiola commented 3 days ago

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 commented 3 days ago

cc @WordPress/gutenberg-core

youknowriad commented 3 days ago

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 commented 2 days ago

@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 commented 2 days ago

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 commented 2 days ago

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

gziolo commented 2 days ago

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

DaniGuardiola commented 2 days ago

@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.