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

Fix small uppercase/lowercase inconsistency in create block template #39853

Closed

Conversation

luisherranz
Copy link
Member

What?

Fix a small inconsistency with uppercase/lowercase of Edit and Save components in the create-block template.

Why?

It felt a bit weird to use lowercase for save and uppercase for Edit. Maybe people can think that it is necessary for some reason.

How?

I've renamed save to Save to follow React's standard, but it could be done the other way around (Edit to edit).

@gziolo
Copy link
Member

gziolo commented Mar 29, 2022

It's a recurring issue. Edit needs to be uppercase because ESLint rules for React components trigger errors. save technically isn't a React component (you can't use React lifecycle methods, react hooks, but you can use some React components and Context API) but a function call that produces HTML string. I'm inclined to use Save and Edit to make the linter happy and remove all the confusion it creates. Both @ryanwelcher and @mkaz opened similar PRs in the past to use the consistent casing for those two API methods that sadly are lowercased inside the block settings.

If we agree to apply the changes proposed we should also update the Create a Block Tutorial and other templates used when scaffoling blocks:

@gziolo gziolo added [Package] Create Block /packages/create-block [Type] Code Quality Issues or PRs that relate to code quality labels Mar 29, 2022
@luisherranz
Copy link
Member Author

save technically isn't a React component

Oh, I see your point 🙂

But I'm not sure if people will infer that they cannot use hooks from the fact that it's lowercase instead of uppercase. I think people see JSX and they assume it is a proper component.

you can't use React lifecycle methods, react hooks, but you can use some React components and Context API

Could you please explain to me what is the technical problem with hooks? Why are the save functions different than regular server-side-rendered React components?

@gziolo
Copy link
Member

gziolo commented Mar 31, 2022

But I'm not sure if people will infer that they cannot use hooks from the fact that it's lowercase instead of uppercase. I think people see JSX and they assume it is a proper component.

Yes, it always has been confusing and this issue existed from the very beginning of the project.

you can't use React lifecycle methods, react hooks, but you can use some React components and Context API

Could you please explain to me what is the technical problem with hooks? Why are the save functions different than regular server-side-rendered React components?

I explained it in this comment #38139 (comment). The gist of it is that save uses internally a custom implementation of renderToString that differs from the one that React uses. WordPress core has different opinions about the markup that conflict with what React produces so this was a way to make those two work seamlessly together.

Regarding React hooks, the implementation was never added to the custom serializer. @nerrad started looking into that in #15873, but it didn't seem like a very pressing issue back then since the initial; idea was to make those hooks ignored anyway.

@luisherranz
Copy link
Member Author

luisherranz commented Mar 31, 2022

Thanks, @gziolo. I understand that Gutenberg is using a custom renderer, but I'd like to understand why it needs it, and why the save components are different than regular server-side-rendered React components, especially if we want to start sharing code between the save and the frontend components.

But this is probably not the place to talk about it.

@gziolo gziolo deleted the fix/create-block-template-uppercase-inconsistency branch March 31, 2022 09:23
@gziolo
Copy link
Member

gziolo commented Mar 31, 2022

I wanted to emphasize that it's 3rd or even 4th attempt to unify the casing for those two names from folks working closely with blocks. It's a clear sign that we need to finally do something about it because it is simply too confusing for everyone.

I would appreciate some help here from @ryanwelcher, @juanmaguitar, and @mburridge who to my knowledge are actively looking into ways to improve the block development experience.

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

Successfully merging this pull request may close these issues.

None yet

2 participants