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

Interpolate Component Experiment #1 #16374

Closed
wants to merge 3 commits into from

Conversation

nerrad
Copy link
Contributor

@nerrad nerrad commented Jul 1, 2019

Description

This is the initial experiment prompted by #9846.

In progress towards solving the problem posed in #9846, this pull was created with the following goals in mind:

  • expose a simple api for wrapping react elements that could be easily localized.
  • re-use as much of the current i18n tools as possible (extraction, pluralization etc) (recognizing that there still may be some work needed)
  • expose a utility that can have purpose beyond string localization
  • expose an api that doesn't require a build process for developers in the WordPress ecosystem that want to use interpolation for localizable strings.
  • Translateable strings should retain context for translators.

In this pull:

Introduction of createInterpolateElement.

This function works similarly to createElement in exposing an api for creating a WordPress (React) element.

It accepts two arguments:

Argument Type description
interpolatedString string The interpolation string to be parsed. This can be wrapped with i18n functions.
conversionMap Array[] The map used to convert the string to a react element.

Example:

const MyCustomComponent = ( { url } ) => {
	return createInterpolateComponent(
		__( 'This is a <span%1>string</span%1> with <a%1>a link</a%1>, a self-closing %1$s component and a plain value: %2$s' ),
		[
			[ 'span%1', { tag: CustomComponent, props: {}, hasChildren: true } ],
			[ 'a%1', { tag: 'a', props: { href: url }, hasChildren: true } ],
			[ '%1$s', { tag: CustomComponentB, props: {}, hasChildren: false } ],
			[ '%2$s', { value: 'custom value' } ],
		]
	);
};

The function returns this as the equivalent:

import { __ } from '@wordpress/i18n';
import { createInterpolateComponent, createElement } from '@wordpress/element';
import { CustomComponent, CustomComponentB } from './custom-components';

const MyCustomComponent = ( { url } ) => {
	return createElement(
		Fragment,
		{},
		[
			'This is a ',
			createElement(
				CustomComponent,
				{ key: 1 },
				'string'
			),
			' with ',
			createElement(
				'a',
				{ key: 2, href: url },
				'a link'
			),
			', a self-closing ',
			createElement(
				CustomComponentB,
				{ key: 3 },
			),
			' component and a plain value: ',
			createElement(
				Fragment,
				{ key: 4 },
				'custom value'
			),
		],
	);
};

This allows for usage immediately wherever interpolation is needed for translated strings.

Some notes:

Tags can be virtually anything, but for the purpose of localization it's recommended to use something that provides context for translators.

The conversion map is expected to be an array of configuration elements ordered by how the tags appear in the interpolation string. Each element in the array is expected to have two values:

  • The first value is the tag that that will be matched in the string.
  • The second value is either a component configuration object with tag, props, and hasChildren keys, or a "prop value" type object with value as the key. The former is similar to the arguments for wp.element.createElement except rather then the third being an array of children, it is simply a boolean indicating whether the given tag represents an element with children. This is because children are extracted from the interpolation string rather than being predefined and may be nested elements. The latter simply signals that the given search string is a straight replacement with the given value.

This api is a bit verbose and could be prone to human error, however it solves the majority of the issues with the current problem out of the box and allows for immediate use.

Interpolate component

This component is currently a demonstration of how an easier api could be exposed to developers for writing interpolation type components.

Usage:

Here's an example of how it could be used (using the same string as our example):

const MyCustomComponent = ( { url } ) => {
	return <Interpolate custom={ 'custom value' }>
		This is a <CustomComponent>string</CustomComponent> with <a href={ url }>a link</a>, a self-closing <CustomComponentB /> component and a plain value: %%custom%%
	</Interpolate>
};

This will result in the same element as described in the previous example. Note the differences:

  • The content can be written more naturally using familiar api.
  • Dynamic values that should be evaluated at runtime are indicated via special %%value%% syntax and the value should correspond to the name of the prop defined on the Interpolate tag. The format of this syntax is negotiable but for the sake of this pull I used something that should be fairly straightforward.

Internally, Interpolate calls createInterpolateElement and uses the new function, createInterpolateString to generate the interpolation string from the element. The created string for this example is the same as what was fed into createInterpolateElement in the previous example. Currently in the code this is not localized but this gives an example of how it could be so that Interpolate could be a suitable friendlier api for this use-case.

Next steps

I've mostly submitted this pull to demonstrate what I think will be a good path to go forward with in solving #9846. Here's some next steps to keep momentum:

  • Evaluate this approach. What works well? what needs changed?
  • I think at a minimum createInterpolateElement seems to best fit as an export on @wordpress/element however it's arguable whether Interpolate should live here also or be exported from @wordpress/i18n. Typical usage will likely be for i18n so it'd probably be better to put Interpolate there.
  • Naming wise, I think it might be better for the component to be named something like <Translate> or something like that. Feasibly in the future it could serve as the mechanism (via context hooks) for live language switching.
  • The component currently doesn't localize or account for singular/plural form. I think the api described in i18n: Implement a solution for interpolation of React elements in strings #9846 for that would be suitable (have Translate.Singular and Translate.Plural) and should be fairly trivial to detect and implement. For actual localization, I think the best method would be to write a babel plugin that translates <Translate>{ children }</Translate> to use createInterpolateElement with the appropriate wp.i18n function wrapping the interpolationString argument on the built code. That would allow all existing string extraction methods to "just" work against built code as before.

There's probably some other thoughts I've missed here, I'll add them as I remember, but for now I think this is ready for initial feedback, review and direction. It is not in mergeable state yet , however we could decide to extract createInterpolateElement into its own pull as it could be released as is for a good interim solution.

How has this been tested?

There are unit tests covering the public api and demonstrating expected behaviour.

Types of changes

This is a new feature introduced to solve problems outlined in #9846

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@nerrad nerrad added [Package] Element /packages/element [Type] Enhancement A suggestion for improvement. Internationalization (i18n) Issues or PRs related to internationalization efforts labels Jul 1, 2019
@nerrad nerrad self-assigned this Jul 1, 2019
*/
const Interpolate = ( props ) => {
return createInterpolateElement(
createInterpolateString( props.children, props ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As noted in the pull description, right now this isn't localizing anything, but it does demonstrate how a localizable string can be built using this new helper method. I think if we decide to expose the nicer api via a component, we should use babel to transform the props.children to the interpolation string and output it in the built code. I haven't written a babel plugin yet so that's something I might need help with.

@nerrad
Copy link
Contributor Author

nerrad commented Sep 3, 2019

Todo notes after slack chat

  • extract createInterpolateComponent to its own pull.
  • consider changing second argument to be an object instead of an array of arrays.
  • consider removing the necessity for a hasChildren key in the config (instead detecting via regex).

This is the initial push for the interpolate experiment.

Exports are:

- Interpolate custom component.
- createInterpolateElement (similar to `createElement`).
@nerrad nerrad force-pushed the FET/interpolate-translations-exp1 branch from e7c7897 to cf1b969 Compare September 8, 2019 21:39
@nerrad
Copy link
Contributor Author

nerrad commented Sep 8, 2019

Huh, tests started failing because flatMap is no longer defined. This was working in the original pull so I suspect an environment change (possibly babel no longer doing the compile for jest?)

@nerrad
Copy link
Contributor Author

nerrad commented Sep 8, 2019

Okay so I think I know why tests fail.

Not certain what should be done here. It seems to me we should be targeting builds that will be similar to the browser builds (with appropriate polyfills etc) right? cc @gziolo.

For now, I'll just switch to lodash _.flatMap. Although I'm trying to stay away from adding lodash dependencies here, there's already a dependency on lodash.escapeRegEx.

@nerrad
Copy link
Contributor Author

nerrad commented Nov 17, 2019

now that #17376 has been merged, this can be closed. Future iterations can incorporate some of the ideas around a <Translate> wrapper etc.

@nerrad nerrad closed this Nov 17, 2019
@nerrad nerrad deleted the FET/interpolate-translations-exp1 branch November 17, 2019 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Package] Element /packages/element [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant