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

Migrate all React functionality to use @wordpress/element with dependency extraction #2756

Merged
merged 11 commits into from
May 17, 2022
95 changes: 67 additions & 28 deletions assets/css/synonyms.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
:root {
--ep-synonyms-input-border-color: hsl(0, 0%, 80%);
--ep-synonyms-color-black: #1a1e24;
--ep-synonyms-color-error: #b52727;
}

html.wp-toolbar {
background: transparent;
}

#synonym-root {

& .page-title-action {
Expand All @@ -9,68 +19,97 @@
}

& h2 {
color: #23282d;
color: var(--ep-synonyms-color-black);
}
}

.synonym-alternatives-editor,
.synonym-set-editor {
display: flex;
.synonym-editor {

& > .postbox {
& .postbox {
width: 100%;

& > .hndle {
display: flex;
}
}
}

& .synonym-alternative-editor {
display: flex;
}
.synonym-alternative-editor,
.synonym-set-editor {
align-items: flex-start;
display: flex;

& .ep-synonyms__linked-multi-input {
& .components-form-token-field {
flex: 1;
margin-bottom: 0.5em;
}

& .ep-synonyms__input {
border: 1px solid hsl(0, 0%, 80%);
margin-bottom: 0.5em;
margin-right: 1em;
width: 10em;
& .components-form-token-field__label {
display: none;
}

& .synonym-alternatives__primary-heading {
width: 11em;
& .components-form-token-field__input-container {
border-color: var(--ep-synonyms-input-border-color);
box-sizing: border-box;
}

& .synonym-alternatives__input-heading {
flex: 1;
& .components-form-token-field__token-text {
padding-bottom: 1px;
padding-top: 1px;

@media screen and (max-width: 782px) {
padding-bottom: 3px;
padding-top: 3px;
}
}

& .ep-synonyms__linked-multi-input input:focus {
box-shadow: none;
& input[type="text"].components-form-token-field__input {
margin-bottom: 2px;
margin-top: 2px;

@media screen and (max-width: 782px) {
min-height: 30px;
}
}

&.invalid input {
border-color: #a00;
& .components-form-token-field__help {
margin-top: 0;
}
}

.synonym-alternative-editor,
.synonym-set-editor {
margin-top: 0.625em;
input[type="text"].ep-synonyms__input {
border: 1px solid var(--ep-synonyms-input-border-color);
margin-bottom: 0.5em;
margin-right: 1em;
min-height: 36px;
width: 10em;

@media screen and (max-width: 782px) {
min-height: 40px;
}
}

.synonym-alternatives__primary-heading {
width: 11em;
}

.synonym-alternatives__input-heading {
flex: 1;
}

button.synonym__remove {
background-color: transparent;
border: none;
color: #a00;
color: var(--ep-synonyms-color-error);
cursor: pointer;
margin: -8px 0 0 10px;
margin: 0 0 0 10px;
min-height: 36px;
padding: 0;

@media screen and (max-width: 782px) {
min-height: 40px;
}

& .dashicons-dismiss {
margin: -2px 2px 0 0;
}
Expand All @@ -84,7 +123,7 @@ button.synonym__remove {

.synonym__validation,
.synonym-solr-editor__validation p {
color: #a00;
color: var(--ep-synonyms-color-error);
font-style: italic;
}

Expand Down
25 changes: 14 additions & 11 deletions assets/js/autosuggest.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
/* eslint-disable camelcase, no-underscore-dangle, no-use-before-define */

/**
* WordPress dependencies.
*/
import apiFetch from '@wordpress/api-fetch';
Copy link
Member

@nicholasio nicholasio May 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: @wordpress/api-fetch will force loading lodash on the front-end which is a huge dep. I think you should stick to fetch on the front-end.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also recommend looking into this: https://github.com/10up/10up-toolkit/blob/develop/packages/toolkit/README.md#-react--wordpress

There are a few things you can prob dequeue on the front-end (assuming IE 11 is not supported anymore)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicholasio Thanks for the tip re: fetch.

Regarding dequeuing the polyfill, we aren't supporting IE11 ourselves, but it looks like the suggestions would remove the polyfill as a dependency of wp-element for all themes and plugins that use it, not just ElasticPress, and I don't think that's something we'd want to do as a publicly distributed plugin. Thoughts @felipeelia?

We could remove wp-polyfill as a dependency of our scripts in the get_asset_info() function, but if it's a dependecy of wp-element anyway there's probably not much point.

Open to any other ideas though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the change introduced in this PR start to enqueue lodash and wp-polyfill or is that something that is already happening?

Curious to hear from @nicholasio about that anyway. It sounds like if someone out there is relying on those packages and we simply dequeue them in the name of performance, things will break. In that case, we won't be touching it in this release.

In general, that sounds more like an optimization that site owners should apply rather than something we could make in the plugin -- although the performance gain would be generally more significant for most of the sites, we obviously will only hear from the sites it actually broke what was already working before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are both right, doing this optimization on the plugin wouldn't make sense! I'd however try to stick with standard apis on the front-end. Most of the @wordpress/packages are not optimized for usage outise of the editor. Most of them will load lodash (and some will load moment.js) both well know to be huge packages.

Copy link
Member

@nicholasio nicholasio May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the very least I'd move away from @wordpress/date as @wordpress/element can be optimized by site owners that do not support IE 11 (and I know wordpress core is trying to optimize the polyfills they add as well)

Moving away from @wordpress/element means shipping react which could cause multiple react to be enqueed on the front-end if other plugins are also relying on react or wordpress/element.

Copy link
Contributor Author

@JakePT JakePT May 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicholasio @wordpress/date does things that we need, namely take raw date data and display it in the site's timezone and language in JS. In the future we'd probably also want to let site owners or devs use their own date format as well. Would you recommend a different approach?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are smaller libraries tath can do what you need. Look at daysjs https://day.js.org/docs/en/plugin/utc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicholasio Ok cool we'll check that out. @felipeelia In the meantime, this PR isn't introducing that package so it probably doesn't need to hold this up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've filed an issue to look into those packages #2768


/**
* Internal dependencies.
*/
import {
findAncestorByClass,
escapeDoubleQuotes,
replaceGlobally,
debounce,
domReady,
} from './utils/helpers';
import 'element-closest';
import 'promise-polyfill/src/polyfill';
import 'whatwg-fetch';

const { epas } = window;

Expand Down Expand Up @@ -167,11 +173,13 @@ function buildSearchQuery(searchText, placeholder, { query }) {
async function esSearch(query, searchTerm) {
const fetchConfig = {
body: query,
method: 'POST',
mode: 'cors',
credentials: 'omit',
headers: {
'Content-Type': 'application/json; charset=utf-8',
},
method: 'POST',
mode: 'cors',
url: epas.endpointUrl,
};

if (epas?.http_headers && typeof epas.http_headers === 'object') {
Expand All @@ -186,12 +194,7 @@ async function esSearch(query, searchTerm) {
}

try {
const response = await window.fetch(epas.endpointUrl, fetchConfig);
if (!response.ok) {
throw Error(response.statusText);
}

const data = await response.json();
const data = await apiFetch(fetchConfig);

// allow for filtered data before returning it to
// be output on the front end
Expand Down
17 changes: 8 additions & 9 deletions assets/js/blocks/related-posts/Edit.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
const { __ } = wp.i18n;

const { AlignmentToolbar, BlockControls, InspectorControls } = wp.editor;

const { PanelBody, Placeholder, Spinner, QueryControls } = wp.components;

const { Fragment, Component, RawHTML } = wp.element;

const { addQueryArgs } = wp.url;
/**
* WordPress dependencies.
*/
import { AlignmentToolbar, BlockControls, InspectorControls } from '@wordpress/block-editor';
import { PanelBody, Placeholder, Spinner, QueryControls } from '@wordpress/components';
import { Fragment, Component, RawHTML } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { addQueryArgs } from '@wordpress/url';

/**
* Edit component
Expand Down
13 changes: 9 additions & 4 deletions assets/js/blocks/related-posts/block.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import Edit from './Edit';

const { __ } = wp.i18n;
/**
* WordPress dependencies.
*/
import { registerBlockType } from '@wordpress/blocks';
import { __ } from '@wordpress/i18n';

const { registerBlockType } = wp.blocks;
/**
* Internal dependencies.
*/
import Edit from './Edit';

registerBlockType('elasticpress/related-posts', {
title: __('Related Posts (ElasticPress)', 'elasticpress'),
Expand Down
10 changes: 8 additions & 2 deletions assets/js/ordering/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import ReactDOM from 'react-dom';
/**
* WordPress dependencies.
*/
import { render } from '@wordpress/element';

/**
* Internal dependencies.
*/
import { Pointers } from './pointers';

ReactDOM.render(<Pointers />, document.getElementById('ordering-app'));
render(<Pointers />, document.getElementById('ordering-app'));
20 changes: 15 additions & 5 deletions assets/js/ordering/pointers.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
// External
import React, { Component } from 'react';
import apiFetch from '@wordpress/api-fetch';
/**
* External dependencies.
*/
import { DragDropContext, Droppable, Draggable } from 'react-beautiful-dnd';

/**
* WordPress dependencies.
*/
import apiFetch from '@wordpress/api-fetch';
import { Component, Fragment } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies.
*/
import { pluck, debounce } from '../utils/helpers';

apiFetch.use(apiFetch.createRootURLMiddleware(window.epOrdering.restApiRoot));
Expand Down Expand Up @@ -369,7 +379,7 @@ export class Pointers extends Component {
);

return (
<React.Fragment key={item.ID}>
<Fragment key={item.ID}>
{parseInt(window.epOrdering.postsPerPage, 10) ===
index && (
<Draggable
Expand Down Expand Up @@ -450,7 +460,7 @@ export class Pointers extends Component {
</div>
)}
</Draggable>
</React.Fragment>
</Fragment>
);
})}
{provided.placeholder}
Expand Down
13 changes: 10 additions & 3 deletions assets/js/synonyms/components/SynonymsEditor.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import React, { useContext, useEffect } from 'react';
/**
* WordPress dependencies.
*/
import { useContext, useEffect, WPElement } from '@wordpress/element';

/**
* Internal dependencies.
*/
import { State, Dispatch } from '../context';
import SetsEditor from './editors/SetsEditor';
import AlterativesEditor from './editors/AlternativesEditor';
import SetsEditor from './editors/SetsEditor';
import SolrEditor from './editors/SolrEditor';

/**
* Synonyms editor component.
*
* @returns {React.FC} Synonyms component
* @returns {WPElement} Synonyms component
*/
const SynonymsEditor = () => {
const state = useContext(State);
Expand Down
17 changes: 12 additions & 5 deletions assets/js/synonyms/components/editors/AlternativeEditor.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import React, { useState, useEffect, useContext, useRef } from 'react';
import LinkedMultiInput from '../shared/LinkedMultiInput';
/**
* WordPress dependencies.
*/
import { useContext, useEffect, useMemo, useRef, useState, WPElement } from '@wordpress/element';

/**
* Internal dependencies.
*/
import { Dispatch } from '../../context';
import LinkedMultiInput from '../shared/LinkedMultiInput';

/**
* Alternative Editor
*
* @param {object} props Props.
* @returns {React.FC} AlternativeEditor component
* @returns {WPElement} AlternativeEditor component
*/
const AlternativeEditor = (props) => {
const { id, synonyms, removeAction, updateAction } = props;
Expand All @@ -32,7 +39,7 @@ const AlternativeEditor = (props) => {
/**
* Handle key down.
*
* @param {React.SyntheticEvent} event Keydown event.
* @param {Event} event Keydown event.
*/
const handleKeyDown = (event) => {
switch (event.key) {
Expand All @@ -54,7 +61,7 @@ const AlternativeEditor = (props) => {
primaryRef.current.focus();
}, [primaryRef]);

const memoizedSynonyms = React.useMemo(() => {
const memoizedSynonyms = useMemo(() => {
return synonyms.filter((item) => !item.primary);
}, [synonyms]);

Expand Down
15 changes: 11 additions & 4 deletions assets/js/synonyms/components/editors/AlternativesEditor.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
import React, { Fragment, useContext } from 'react';
import AlternativeEditor from './AlternativeEditor';
/**
* WordPress dependencies.
*/
import { Fragment, useContext, WPElement } from '@wordpress/element';

/**
* Internal dependencies.
*/
import { Dispatch, State } from '../../context';
import AlternativeEditor from './AlternativeEditor';

/**
* Synonyms editor component.
*
* @param {object} props Props.
* @param {object[]} props.alternatives Defined alternatives (explicit mappings).
* @returns {React.FC} AlternativesEditor component
* @returns {WPElement} AlternativesEditor component
*/
const AlternativesEditor = ({ alternatives }) => {
const dispatch = useContext(Dispatch);
Expand All @@ -22,7 +29,7 @@ const AlternativesEditor = ({ alternatives }) => {
/**
* Handle click.
*
* @param {React.SyntheticEvent} e Event.
* @param {Event} e Event.
*/
const handleClick = (e) => {
const [lastItem] = state.alternatives.slice(-1);
Expand Down
Loading