From e2421b1666f83315850f5fb68e4ec9bfdd080f50 Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Tue, 14 Jan 2025 09:48:15 +0200 Subject: [PATCH 1/4] add helper to catch errors for interpolated strings Also implements it in the site basic page tests for i18nCreateInterpolateElement fix lint restore testURL remove jsdom --- packages/helpers/package.json | 1 + .../src/i18n-create-interpolate-element.js | 24 +++++++++ packages/helpers/src/index.js | 1 + .../tests/i18nCreateInterpolateElementTest.js | 52 +++++++++++++++++++ .../js/src/settings/routes/site-basics.js | 20 +++---- yarn.lock | 21 ++++++++ 6 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 packages/helpers/src/i18n-create-interpolate-element.js create mode 100644 packages/helpers/tests/i18nCreateInterpolateElementTest.js diff --git a/packages/helpers/package.json b/packages/helpers/package.json index 0e349084362..fbc5b7e1840 100644 --- a/packages/helpers/package.json +++ b/packages/helpers/package.json @@ -23,6 +23,7 @@ "license": "GPL-3.0", "private": false, "dependencies": { + "@wordpress/element": "^6.14.0", "@wordpress/i18n": "^1.2.3", "lodash": "^4.17.21", "prop-types": "^15.7.2", diff --git a/packages/helpers/src/i18n-create-interpolate-element.js b/packages/helpers/src/i18n-create-interpolate-element.js new file mode 100644 index 00000000000..9a33c2be7c8 --- /dev/null +++ b/packages/helpers/src/i18n-create-interpolate-element.js @@ -0,0 +1,24 @@ +import { createInterpolateElement } from "@wordpress/element"; +import { sprintf } from "@wordpress/i18n"; + +/** + * Create an interpolated element from a translated string and catches errors. + * + * @param {string} translatedString The translated string to be interpolated. + * @param {[string]} args The arguments to be interpolated into the translated string. + * @param {object} conversionMap The conversion map for the interpolated values. + * + * @returns {object} The interpolated element. + */ +export const i18nCreateInterpolateElement = ( translatedString, args, conversionMap ) => { + try { + return createInterpolateElement( + sprintf( + translatedString, + ...args + ), conversionMap ); + } catch ( error ) { + console.error( "Error in translation for:", translatedString ); + return translatedString; + } +}; diff --git a/packages/helpers/src/index.js b/packages/helpers/src/index.js index d3b9d27da3e..d83df6417dc 100644 --- a/packages/helpers/src/index.js +++ b/packages/helpers/src/index.js @@ -26,3 +26,4 @@ export { makeOutboundLink } from "./makeOutboundLink"; export { default as validateFacebookImage } from "./social-preview-image-validation/facebookValidation"; export { default as validateTwitterImage } from "./social-preview-image-validation/twitterValidation"; export * from "./hiddenInputHelper"; +export * from "./i18n-create-interpolate-element"; diff --git a/packages/helpers/tests/i18nCreateInterpolateElementTest.js b/packages/helpers/tests/i18nCreateInterpolateElementTest.js new file mode 100644 index 00000000000..71cd83eb843 --- /dev/null +++ b/packages/helpers/tests/i18nCreateInterpolateElementTest.js @@ -0,0 +1,52 @@ +import React from "react"; +import { i18nCreateInterpolateElement } from "../src/i18n-create-interpolate-element"; +import { createInterpolateElement } from "@wordpress/element"; +import { sprintf } from "@wordpress/i18n"; + +jest.mock( "@wordpress/element", () => ( { + createInterpolateElement: jest.fn(), +} ) ); + +jest.mock( "@wordpress/i18n", () => ( { + sprintf: jest.fn(), +} ) ); + +describe( "i18nCreateInterpolateElement", () => { + beforeEach( () => { + jest.clearAllMocks(); + } ); + + it( "should return interpolated element when no error occurs", () => { + const translatedString = "Hello %s"; + const args = [ "world" ]; + const conversionMap = { world: world }; + const interpolatedElement = Hello world; + + sprintf.mockReturnValue( "Hello world" ); + createInterpolateElement.mockReturnValue( interpolatedElement ); + + const result = i18nCreateInterpolateElement( translatedString, args, conversionMap ); + + expect( sprintf ).toHaveBeenCalledWith( translatedString, ...args ); + expect( createInterpolateElement ).toHaveBeenCalledWith( "Hello world", conversionMap ); + expect( result ).toBe( interpolatedElement ); + } ); + + it( "should return translated string and log error when an error occurs", () => { + const translatedString = "Hello %s"; + const args = [ "world" ]; + const conversionMap = { world: world }; + + sprintf.mockImplementation( () => { + throw new Error( "Test error" ); + } ); + + console.error = jest.fn(); + + const result = i18nCreateInterpolateElement( translatedString, args, conversionMap ); + + expect( sprintf ).toHaveBeenCalledWith( translatedString, ...args ); + expect( console.error ).toHaveBeenCalledWith( "Error in translation for:", translatedString ); + expect( result ).toBe( translatedString ); + } ); +} ); diff --git a/packages/js/src/settings/routes/site-basics.js b/packages/js/src/settings/routes/site-basics.js index 75d723e5bcc..0b93ed63731 100644 --- a/packages/js/src/settings/routes/site-basics.js +++ b/packages/js/src/settings/routes/site-basics.js @@ -15,6 +15,7 @@ import { } from "../components"; import { withDisabledMessageSupport, withFormikDummySelectField } from "../hocs"; import { useDispatchSettings, useSelectSettings } from "../hooks"; +import { i18nCreateInterpolateElement } from "@yoast/helpers"; const ToggleFieldWithDisabledMessageSupport = withDisabledMessageSupport( ToggleField ); const FormikSelectPageWithDummy = withFormikDummySelectField( FormikPageSelectField ); @@ -104,16 +105,15 @@ const SiteBasics = () => { strong: , } ), [] ); - const taglineDescription = useMemo( () => createInterpolateElement( - sprintf( - /** - * translators: %1$s expands to an opening anchor tag. - * %2$s expands to a closing anchor tag. - */ - __( "This field updates the %1$stagline in your WordPress settings%2$s.", "wordpress-seo" ), - "", - "" - ), + + + const taglineDescription = useMemo( () => i18nCreateInterpolateElement( + /** + * translators: %1$s expands to an opening anchor tag. + * %2$s expands to a closing anchor tag. + */ + __( "This field updates the %1$stagline in your WordPress settings%2$s.", "wordpress-seo" ), + [ "", "" ], { // eslint-disable-next-line jsx-a11y/anchor-has-content a: , diff --git a/yarn.lock b/yarn.lock index 5faeb09e595..7d3ecbb0369 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8479,6 +8479,20 @@ react "^18.2.0" react-dom "^18.2.0" +"@wordpress/element@^6.14.0": + version "6.14.0" + resolved "https://registry.yarnpkg.com/@wordpress/element/-/element-6.14.0.tgz#d29a837d7a01c62d206bb2ae44c27a4f199736c1" + integrity sha512-vZPm2ekv9B7fMcv/slyu/p8lV44EPa6RRHOk04ldNUpsrjC6ph6Q4wpuI5WzLEX7p1u71c8ZOuroEuRvdFxMcA== + dependencies: + "@babel/runtime" "7.25.7" + "@types/react" "^18.2.79" + "@types/react-dom" "^18.2.25" + "@wordpress/escape-html" "*" + change-case "^4.1.2" + is-plain-object "^5.0.0" + react "^18.3.0" + react-dom "^18.3.0" + "@wordpress/element@^6.7.0": version "6.7.0" resolved "https://registry.yarnpkg.com/@wordpress/element/-/element-6.7.0.tgz#8d8ac6568909135b8b86131ee689a2724f36df05" @@ -8493,6 +8507,13 @@ react "^18.3.0" react-dom "^18.3.0" +"@wordpress/escape-html@*": + version "3.14.0" + resolved "https://registry.yarnpkg.com/@wordpress/escape-html/-/escape-html-3.14.0.tgz#ef7007b4edc71a3b00b93bc1870057ae4ddad2ea" + integrity sha512-tLzQk7VQse1TF/StFe6vt4zdPCWV9LFRPhseC46tbBxAlm/+v6gmaJP501voA/vPQOJSZrYyA5iXGQhA8cJsRw== + dependencies: + "@babel/runtime" "7.25.7" + "@wordpress/escape-html@^1.12.2": version "1.12.2" resolved "https://registry.yarnpkg.com/@wordpress/escape-html/-/escape-html-1.12.2.tgz#dcc92178bacc69952cde9bb8fb1cbbea9deb2cc3" From 3500d3cd5c49328cc7b0b5338e2c62f28164f4ae Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Tue, 14 Jan 2025 09:48:41 +0200 Subject: [PATCH 2/4] Revert "add helper to catch errors for interpolated strings" This reverts commit e2421b1666f83315850f5fb68e4ec9bfdd080f50. --- packages/helpers/package.json | 1 - .../src/i18n-create-interpolate-element.js | 24 --------- packages/helpers/src/index.js | 1 - .../tests/i18nCreateInterpolateElementTest.js | 52 ------------------- .../js/src/settings/routes/site-basics.js | 20 +++---- yarn.lock | 21 -------- 6 files changed, 10 insertions(+), 109 deletions(-) delete mode 100644 packages/helpers/src/i18n-create-interpolate-element.js delete mode 100644 packages/helpers/tests/i18nCreateInterpolateElementTest.js diff --git a/packages/helpers/package.json b/packages/helpers/package.json index fbc5b7e1840..0e349084362 100644 --- a/packages/helpers/package.json +++ b/packages/helpers/package.json @@ -23,7 +23,6 @@ "license": "GPL-3.0", "private": false, "dependencies": { - "@wordpress/element": "^6.14.0", "@wordpress/i18n": "^1.2.3", "lodash": "^4.17.21", "prop-types": "^15.7.2", diff --git a/packages/helpers/src/i18n-create-interpolate-element.js b/packages/helpers/src/i18n-create-interpolate-element.js deleted file mode 100644 index 9a33c2be7c8..00000000000 --- a/packages/helpers/src/i18n-create-interpolate-element.js +++ /dev/null @@ -1,24 +0,0 @@ -import { createInterpolateElement } from "@wordpress/element"; -import { sprintf } from "@wordpress/i18n"; - -/** - * Create an interpolated element from a translated string and catches errors. - * - * @param {string} translatedString The translated string to be interpolated. - * @param {[string]} args The arguments to be interpolated into the translated string. - * @param {object} conversionMap The conversion map for the interpolated values. - * - * @returns {object} The interpolated element. - */ -export const i18nCreateInterpolateElement = ( translatedString, args, conversionMap ) => { - try { - return createInterpolateElement( - sprintf( - translatedString, - ...args - ), conversionMap ); - } catch ( error ) { - console.error( "Error in translation for:", translatedString ); - return translatedString; - } -}; diff --git a/packages/helpers/src/index.js b/packages/helpers/src/index.js index d83df6417dc..d3b9d27da3e 100644 --- a/packages/helpers/src/index.js +++ b/packages/helpers/src/index.js @@ -26,4 +26,3 @@ export { makeOutboundLink } from "./makeOutboundLink"; export { default as validateFacebookImage } from "./social-preview-image-validation/facebookValidation"; export { default as validateTwitterImage } from "./social-preview-image-validation/twitterValidation"; export * from "./hiddenInputHelper"; -export * from "./i18n-create-interpolate-element"; diff --git a/packages/helpers/tests/i18nCreateInterpolateElementTest.js b/packages/helpers/tests/i18nCreateInterpolateElementTest.js deleted file mode 100644 index 71cd83eb843..00000000000 --- a/packages/helpers/tests/i18nCreateInterpolateElementTest.js +++ /dev/null @@ -1,52 +0,0 @@ -import React from "react"; -import { i18nCreateInterpolateElement } from "../src/i18n-create-interpolate-element"; -import { createInterpolateElement } from "@wordpress/element"; -import { sprintf } from "@wordpress/i18n"; - -jest.mock( "@wordpress/element", () => ( { - createInterpolateElement: jest.fn(), -} ) ); - -jest.mock( "@wordpress/i18n", () => ( { - sprintf: jest.fn(), -} ) ); - -describe( "i18nCreateInterpolateElement", () => { - beforeEach( () => { - jest.clearAllMocks(); - } ); - - it( "should return interpolated element when no error occurs", () => { - const translatedString = "Hello %s"; - const args = [ "world" ]; - const conversionMap = { world: world }; - const interpolatedElement = Hello world; - - sprintf.mockReturnValue( "Hello world" ); - createInterpolateElement.mockReturnValue( interpolatedElement ); - - const result = i18nCreateInterpolateElement( translatedString, args, conversionMap ); - - expect( sprintf ).toHaveBeenCalledWith( translatedString, ...args ); - expect( createInterpolateElement ).toHaveBeenCalledWith( "Hello world", conversionMap ); - expect( result ).toBe( interpolatedElement ); - } ); - - it( "should return translated string and log error when an error occurs", () => { - const translatedString = "Hello %s"; - const args = [ "world" ]; - const conversionMap = { world: world }; - - sprintf.mockImplementation( () => { - throw new Error( "Test error" ); - } ); - - console.error = jest.fn(); - - const result = i18nCreateInterpolateElement( translatedString, args, conversionMap ); - - expect( sprintf ).toHaveBeenCalledWith( translatedString, ...args ); - expect( console.error ).toHaveBeenCalledWith( "Error in translation for:", translatedString ); - expect( result ).toBe( translatedString ); - } ); -} ); diff --git a/packages/js/src/settings/routes/site-basics.js b/packages/js/src/settings/routes/site-basics.js index 0b93ed63731..75d723e5bcc 100644 --- a/packages/js/src/settings/routes/site-basics.js +++ b/packages/js/src/settings/routes/site-basics.js @@ -15,7 +15,6 @@ import { } from "../components"; import { withDisabledMessageSupport, withFormikDummySelectField } from "../hocs"; import { useDispatchSettings, useSelectSettings } from "../hooks"; -import { i18nCreateInterpolateElement } from "@yoast/helpers"; const ToggleFieldWithDisabledMessageSupport = withDisabledMessageSupport( ToggleField ); const FormikSelectPageWithDummy = withFormikDummySelectField( FormikPageSelectField ); @@ -105,15 +104,16 @@ const SiteBasics = () => { strong: , } ), [] ); - - - const taglineDescription = useMemo( () => i18nCreateInterpolateElement( - /** - * translators: %1$s expands to an opening anchor tag. - * %2$s expands to a closing anchor tag. - */ - __( "This field updates the %1$stagline in your WordPress settings%2$s.", "wordpress-seo" ), - [ "", "" ], + const taglineDescription = useMemo( () => createInterpolateElement( + sprintf( + /** + * translators: %1$s expands to an opening anchor tag. + * %2$s expands to a closing anchor tag. + */ + __( "This field updates the %1$stagline in your WordPress settings%2$s.", "wordpress-seo" ), + "", + "" + ), { // eslint-disable-next-line jsx-a11y/anchor-has-content a: , diff --git a/yarn.lock b/yarn.lock index 7d3ecbb0369..5faeb09e595 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8479,20 +8479,6 @@ react "^18.2.0" react-dom "^18.2.0" -"@wordpress/element@^6.14.0": - version "6.14.0" - resolved "https://registry.yarnpkg.com/@wordpress/element/-/element-6.14.0.tgz#d29a837d7a01c62d206bb2ae44c27a4f199736c1" - integrity sha512-vZPm2ekv9B7fMcv/slyu/p8lV44EPa6RRHOk04ldNUpsrjC6ph6Q4wpuI5WzLEX7p1u71c8ZOuroEuRvdFxMcA== - dependencies: - "@babel/runtime" "7.25.7" - "@types/react" "^18.2.79" - "@types/react-dom" "^18.2.25" - "@wordpress/escape-html" "*" - change-case "^4.1.2" - is-plain-object "^5.0.0" - react "^18.3.0" - react-dom "^18.3.0" - "@wordpress/element@^6.7.0": version "6.7.0" resolved "https://registry.yarnpkg.com/@wordpress/element/-/element-6.7.0.tgz#8d8ac6568909135b8b86131ee689a2724f36df05" @@ -8507,13 +8493,6 @@ react "^18.3.0" react-dom "^18.3.0" -"@wordpress/escape-html@*": - version "3.14.0" - resolved "https://registry.yarnpkg.com/@wordpress/escape-html/-/escape-html-3.14.0.tgz#ef7007b4edc71a3b00b93bc1870057ae4ddad2ea" - integrity sha512-tLzQk7VQse1TF/StFe6vt4zdPCWV9LFRPhseC46tbBxAlm/+v6gmaJP501voA/vPQOJSZrYyA5iXGQhA8cJsRw== - dependencies: - "@babel/runtime" "7.25.7" - "@wordpress/escape-html@^1.12.2": version "1.12.2" resolved "https://registry.yarnpkg.com/@wordpress/escape-html/-/escape-html-1.12.2.tgz#dcc92178bacc69952cde9bb8fb1cbbea9deb2cc3" From e258374b5f67da64f8818a665c3d52e461a89c44 Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Tue, 14 Jan 2025 13:34:30 +0200 Subject: [PATCH 3/4] refactor i18nCreateInterpolateElement --- packages/js/src/helpers/i18n.js | 17 +++++++++++++++++ packages/js/src/settings/routes/site-basics.js | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/js/src/helpers/i18n.js b/packages/js/src/helpers/i18n.js index cba8b47ef57..dc8332155e5 100644 --- a/packages/js/src/helpers/i18n.js +++ b/packages/js/src/helpers/i18n.js @@ -1,4 +1,5 @@ import { setLocaleData } from "@wordpress/i18n"; +import { createInterpolateElement } from "@wordpress/element"; import { get } from "lodash"; /** @@ -26,3 +27,19 @@ export function setTextdomainL10n( textdomain, l10nNamespace = "wpseoYoastJSL10n setLocaleData( translations, textdomain ); } } + +/** + * Wrapper function for `createInterpolateElement` to catch errors. + * + * @param {string} interpolatedString The interpolated string. + * @param {object} conversionMap The conversion map object. + * @returns {string} The interpolated string. + */ +export const i18nCreateInterpolateElement = ( interpolatedString, conversionMap ) => { + try { + return createInterpolateElement( interpolatedString, conversionMap ); + } catch ( error ) { + console.error( "Error in translation for:", interpolatedString, error ); + return interpolatedString; + } +}; diff --git a/packages/js/src/settings/routes/site-basics.js b/packages/js/src/settings/routes/site-basics.js index 75d723e5bcc..28b457d6635 100644 --- a/packages/js/src/settings/routes/site-basics.js +++ b/packages/js/src/settings/routes/site-basics.js @@ -15,6 +15,7 @@ import { } from "../components"; import { withDisabledMessageSupport, withFormikDummySelectField } from "../hocs"; import { useDispatchSettings, useSelectSettings } from "../hooks"; +import { i18nCreateInterpolateElement } from "../../helpers/i18n"; const ToggleFieldWithDisabledMessageSupport = withDisabledMessageSupport( ToggleField ); const FormikSelectPageWithDummy = withFormikDummySelectField( FormikPageSelectField ); @@ -104,7 +105,7 @@ const SiteBasics = () => { strong: , } ), [] ); - const taglineDescription = useMemo( () => createInterpolateElement( + const taglineDescription = useMemo( () => i18nCreateInterpolateElement( sprintf( /** * translators: %1$s expands to an opening anchor tag. From c99aad0b144b585d2cb4a990326f6713513f7ea5 Mon Sep 17 00:00:00 2001 From: Vraja Das Date: Tue, 14 Jan 2025 15:31:56 +0200 Subject: [PATCH 4/4] rename --- packages/js/src/helpers/i18n.js | 2 +- packages/js/src/settings/routes/site-basics.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/js/src/helpers/i18n.js b/packages/js/src/helpers/i18n.js index dc8332155e5..74dbb5e57f7 100644 --- a/packages/js/src/helpers/i18n.js +++ b/packages/js/src/helpers/i18n.js @@ -35,7 +35,7 @@ export function setTextdomainL10n( textdomain, l10nNamespace = "wpseoYoastJSL10n * @param {object} conversionMap The conversion map object. * @returns {string} The interpolated string. */ -export const i18nCreateInterpolateElement = ( interpolatedString, conversionMap ) => { +export const safeCreateInterpolateElement = ( interpolatedString, conversionMap ) => { try { return createInterpolateElement( interpolatedString, conversionMap ); } catch ( error ) { diff --git a/packages/js/src/settings/routes/site-basics.js b/packages/js/src/settings/routes/site-basics.js index 28b457d6635..36c02d66351 100644 --- a/packages/js/src/settings/routes/site-basics.js +++ b/packages/js/src/settings/routes/site-basics.js @@ -15,7 +15,7 @@ import { } from "../components"; import { withDisabledMessageSupport, withFormikDummySelectField } from "../hocs"; import { useDispatchSettings, useSelectSettings } from "../hooks"; -import { i18nCreateInterpolateElement } from "../../helpers/i18n"; +import { safeCreateInterpolateElement } from "../../helpers/i18n"; const ToggleFieldWithDisabledMessageSupport = withDisabledMessageSupport( ToggleField ); const FormikSelectPageWithDummy = withFormikDummySelectField( FormikPageSelectField ); @@ -105,7 +105,7 @@ const SiteBasics = () => { strong: , } ), [] ); - const taglineDescription = useMemo( () => i18nCreateInterpolateElement( + const taglineDescription = useMemo( () => safeCreateInterpolateElement( sprintf( /** * translators: %1$s expands to an opening anchor tag.