WordPress / gutenberg

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

`react-autosize-textarea` dependency seems to be abandoned; what does this mean for `<PlainText>`? #39619

Open ZebulanStanphill opened 2 years ago

ZebulanStanphill commented 2 years ago

react-autosize-textarea, a dependency of @wordpress/block-editor, appears to be abandoned. It has not received any updates since July 2020, and a PR made by someone on December 2020 to add support for React 17 has been entirely ignored.

As it turns out, there's a very similar package called react-textarea-autosize, which is actively maintained, and appears to be significantly more popular, according to NPM weekly download statistics.

I would make a PR to replace the dependency right now, but there's a bit of a hitch. Of the 2 locations where the <TextareaAutosize> component from the abandoned package is used, one is in the <PlainText> component:

const PlainText = forwardRef( ( { __experimentalVersion, ...props }, ref ) => {
    if ( __experimentalVersion === 2 ) {
        return <EditableText ref={ ref } { ...props } />;
    }

    const { className, onChange, ...remainingProps } = props;

    return (
        <TextareaAutosize
            ref={ ref }
            className={ classnames( 'block-editor-plain-text', className ) }
            onChange={ ( event ) => onChange( event.target.value ) }
            { ...remainingProps }
        />
    );
} );

As you can see, the <PlainText> component forwards most of its props to <TextareaAutosize>, and there are no specific types declared for the props of <PlainText>. As a result, replacing the <TextareaAutosize> of the abandoned react-autosize-textarea with the one from react-textarea-autosize would necessarily constitute a breaking change, since the latter has a slightly different set of custom props.

But speaking of breaking changes, you can also see that our <PlainText> component has an experimental version 2, which simply forwards props to <EditableText> (another component from @wordpress/block-editor), which does not use <TextareaAutosize> at all. So if we were to declare a breaking change for <PlainText>, it would only affect the "v1" of the component. So at that point, you might as well just get rid of the v1 and fully commit to the v2, right?

Well, not exactly, because the experimental version 2 of <PlainText> is still called "experimental"... and it has been for nearly 2 years now. (See #18361 and #21076.)

So... now what? Pinging @ellatrix and @youknowriad since they were involved in the aforementioned 2 PRs.

youknowriad commented 2 years ago

Maybe this could be a two steps project. Can we first make the v2 stable and deprecate v1? That way we'll be reducing the impact of the v1 removal (that can probably happen in a future WP release)

ZebulanStanphill commented 2 years ago

Since <PlainText> v2 just forwards all its logic to <EditableText>, wouldn't it make the most sense to just deprecate <PlainText> as a whole and recommend using <EditableText> instead? It's worth noting that <EditableText> is not currently exported from the package.

Actually, I think the debate of what to do here is why things ended up the way they are. I guess the question is one of naming. Should the "editable text input in which text formatting is not allowed" be called <PlainText>, <EditableText>, or something else entirely?

youknowriad commented 2 years ago

EditableText is just an internal component that serves as a base for both RichText and PlainText. I still prefer the RichText and PlainText naming, EditableText is a bit confusing IMO.

ZebulanStanphill commented 2 years ago

I've started #39817 to stabilize <PlainText> v2 and deprecate v1. However, there's a bit of a roadblock: several places in the project still use v1. Of these places, they all seem to be cases where the component is used as an editor-only auto-resizing textarea, such as the "HTML" view of the Custom HTML block, or the shortcode field of the Shortcode block.

I'm guessing these are all misuses of the component, and we should switch to just using <TextareaAutosize> from the maintained react-textarea-autosize package in those situations. That would be the correct approach, right?

youknowriad commented 2 years ago

I'm guessing these are all misuses of the component, and we should switch to just using from the maintained react-textarea-autosize package in those situations. That would be the correct approach, right?

I don't know, one could argue that both use-cases are valid, in which case we'd need a "plain text" without borders and such and one that is more like a regular text area. could be a variant prop instead of version. I'd love to have @ellatrix's opinion though.

ZebulanStanphill commented 2 years ago

Any update on this situation? One thing I've neglected to mention until now is that this library causes dependency issues since it does not support React 17: https://github.com/buildo/react-autosize-textarea/blob/master/package.json#L74

t-hamano commented 2 years ago

I agree that v2 should be stabilized and v1 deprecated. I came across this issue while researching #41961.

In the core block, there are two blocks where PlainText v1 is used and the tag name is not specified:

In this case, react-autosize-textarea generates a textarea element, but In the site editor, the element overflows and a resize handle is displayed:

edit-site-shortcode

edit-site-html

This is due to the fact that the reset CSS is not applied to the form elements in the site editor.

I initially considered a CSS fix to PlainText v1 as seen in #42889, but I think that migrating to v2 is more reasonable. As a test, I applied PlainText v2 to those two blocks above, and the problems with overflow and resizing have been fixed. (of course, this is because it is editable div, not textarea)

Is it possible to continue this discussion?

ellatrix commented 2 years ago

I don't know, one could argue that both use-cases are valid, in which case we'd need a "plain text" without borders and such and one that is more like a regular text area. could be a variant prop instead of version. I'd love to have @ellatrix's opinion though.

Sure, we could add a variant prop to indicate that you want the borders styled like a regular input field. But underneath it should remain an editable div where the borders are just added with CSS. We should deprecate v1 and remove the text area autoresizing. v2 with the editable div is much easier to handle.

Happy to review a PR!

t-hamano commented 2 years ago

@ellatrix I'm glad I could hear that!

@ZebulanStanphill I think that continuing #39817 is a good way to go, but what do you think?

ZebulanStanphill commented 2 years ago

Due to PlainText v1 still being used in multiple locations as a <textarea> substitute in the codebase, I can't merge #39817 as-is. It seems that implementing @ellatrix's suggestion should happen, and instead of stabilizing the version prop, remove/deprecate it in favor of a variant prop.

I'll try and get around to it this weekend, but if anyone wants to jump in and do it instead, be my guest.

t-hamano commented 1 year ago

@ZebulanStanphill

If you don't mind, I would like to take over your draft PR #39817. If you have any problems, please do not hesitate to tell me 👍

ZebulanStanphill commented 1 year ago

@t-hamano Go ahead! I'm pretty busy at the moment, so I appreciate someone else picking up the work.

t-hamano commented 1 year ago

I have begun to work on this issue and have run into one problem.

The custom HTML block and the shortcode block are currently using version 1 PlainText. That is, they use the unmaintained react-autosize-textarea package. The contents of these two blocks contain HTML tags, but changing the PlainText version to 2 will remove the HTML tags. I believe this is because the EditableText (i.e., RichText) used for PlainText version 2 does not allow HTML.

Therefore, I believe that either of the following approaches may need to be taken:

I would like to hear your opinion on what approach would be appropriate.

t-hamano commented 1 year ago

I found #48215 which switches aa to #48215.

The custom HTML block and the shortcode block are currently using version 1 PlainText. That is, they use the unmaintained react-autosize-textarea package. The contents of these two blocks contain HTML tags, but changing the PlainText version to 2 will remove the HTML tags. I believe this is because the EditableText (i.e., RichText) used for PlainText version 2 does not allow HTML.

The above issue I was concerned about may be resolved once this PR is merged: textareas that need to keep HTML tags will be replaced directly with the new react-textarea-autosize rather than migrating to PlainText version 2 Approach.

ellatrix commented 11 months ago

We should probably add a prop encodeEntities=false which would not strip HTML tags, and also output them back unencoded. Currently it assumes the text will be used in HTML, so they are encoded.

tyxla commented 2 months ago

I've experimented with removing react-autosize-textarea in favor of our built-in TextareaControl with field-sizing in #64208.

It's a bummer that it's blocked by field-sizing's browser support, but hopefully, this will change soon.