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

Lodash: Remove completely from @wordpress/i18n package #43677

Merged
merged 1 commit into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion packages/i18n/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
"@babel/runtime": "^7.16.0",
"@wordpress/hooks": "file:../hooks",
"gettext-parser": "^1.3.1",
"lodash": "^4.17.21",
"memize": "^1.1.0",
"sprintf-js": "^1.1.1",
"tannin": "^1.2.0"
Expand Down
15 changes: 7 additions & 8 deletions packages/i18n/tools/pot-to-php.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* External dependencies
*/
const gettextParser = require( 'gettext-parser' );
const { isEmpty } = require( 'lodash' );
const fs = require( 'fs' );

const TAB = '\t';
Expand Down Expand Up @@ -47,8 +46,8 @@ function convertTranslationToPHP( translation, textdomain, context = '' ) {
let original = translation.msgid;
const comments = translation.comments;

if ( ! isEmpty( comments ) ) {
if ( ! isEmpty( comments.reference ) ) {
if ( Object.values( comments ).length ) {
if ( comments.reference ) {
Copy link
Member

Choose a reason for hiding this comment

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

This now throws an error if comments is null or undefined

TypeError: Cannot convert undefined or null to object

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch. Fix suggested in #45414.

Test instructions to reproduce would be really helpful to ensure we fixed it properly @manzoorwanijk .

Copy link
Member

Choose a reason for hiding this comment

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

Simply run npx pot-to-php path/to/translations.pot path/to/translations.php your-text-domain to see the error.

// All references are split by newlines, add a // Reference prefix to make them tidy.
php +=
TAB +
Expand All @@ -59,7 +58,7 @@ function convertTranslationToPHP( translation, textdomain, context = '' ) {
NEWLINE;
}

if ( ! isEmpty( comments.translator ) ) {
if ( comments.translator ) {
// All extracted comments are split by newlines, add a tab to line them up nicely.
const translator = comments.translator
.split( NEWLINE )
Expand All @@ -68,7 +67,7 @@ function convertTranslationToPHP( translation, textdomain, context = '' ) {
php += TAB + `/* ${ translator } */${ NEWLINE }`;
}

if ( ! isEmpty( comments.extracted ) ) {
if ( comments.extracted ) {
php +=
TAB + `/* translators: ${ comments.extracted } */${ NEWLINE }`;
}
Expand All @@ -77,8 +76,8 @@ function convertTranslationToPHP( translation, textdomain, context = '' ) {
if ( '' !== original ) {
original = escapeSingleQuotes( original );

if ( isEmpty( translation.msgid_plural ) ) {
if ( isEmpty( context ) ) {
if ( ! translation.msgid_plural ) {
if ( ! context ) {
php += TAB + `__( '${ original }', '${ textdomain }' )`;
} else {
php +=
Expand All @@ -88,7 +87,7 @@ function convertTranslationToPHP( translation, textdomain, context = '' ) {
} else {
const plural = escapeSingleQuotes( translation.msgid_plural );

if ( isEmpty( context ) ) {
if ( ! context ) {
php +=
TAB +
`_n_noop( '${ original }', '${ plural }', '${ textdomain }' )`;
Expand Down