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

npx @wordpress/create-block - text-domain should match block slug not internal namespace #24362

Closed
bobbingwide opened this issue Aug 4, 2020 · 4 comments
Assignees
Labels
[Status] In Progress Tracking issues with work in progress [Tool] Create Block /packages/create-block

Comments

@bobbingwide
Copy link
Contributor

bobbingwide commented Aug 4, 2020

Describe the bug
While the output produced by running npx @wordpress/create-block does successfully create a WordPress plugin and a block that is expected to work, there are several problems with the generated code.

The biggest one is that the generated code doesn't conform to WordPress guidelines.

From https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#text-domains

The text domain must match the slug of the plugin. If your plugin is a single file called my-plugin.php or it is contained in a folder called my-plugin the domain name must be my-plugin. If your plugin is hosted on wordpress.org it must be the slug portion of your plugin URL (wordpress.org/plugins/).

In the generated code the text-domain is set to the same value as the prefix for the block, the internal namespace.
It should be set to the block slug used for identification, the plugin slug.

To reproduce
Steps to reproduce the behavior:

  1. Run npx @wordpress/create-block from the plugins directory.
  2. Complete the responses as shown below.
  3. View the generated source code.
  4. The text domain will be the same as the internal namespace for the block name.
  5. There are also a couple of other problems worth mentioning.

Step 2. Responses to create the 'on-this-day' plugin

C:\apache\htdocs\wordpress\wp-content\plugins>npx @wordpress/create-block
npx: installed 204 in 20.79s
(node:50520) ExperimentalWarning: The fs.promises API is experimental
? The block slug used for identification (also the plugin and output folder name): on-this-day
? The internal namespace for the block name (something unique for your products): oik-sb
? The display title for your block: On this day
? The short description for your block (optional): Display 'On this day' in history related content
? The dashicon to make it easier to identify your block (optional): calendar-alt
? The category name to help users browse and discover your block: widgets
? The name of the plugin author (optional). Multiple authors may be listed using commas: bobbingwide
? The short name of the plugin’s license (optional): GPL-2.0-or-later
? A link to the full text of the license (optional): https://www.gnu.org/licenses/gpl-2.0.html
? The current version number of the plugin: 0.1.0

Creating a new WordPress block in "on-this-day" folder.

Creating a "block.json" file.

Creating a "package.json" file.

Installing packages. It might take a couple of minutes.

Formatting JavaScript files.

Compiling block.

Done: block "On this day" bootstrapped in the "on-this-day" folder.

Expected behavior

  • The text domain is oik-sb. It should be on-this-day, the same name as the plugin.

Other errant/missing behavior that could be improved

  • The single quotes in the short description came out as ' See screenshot.

  • additional work is needed to enable internationalization and localization

    • the PHP code doesn't call wp_set_script_translations.
    • there is no call to load_plugin_textdomain
    • the build commands do not include npm run makepot and npm run makejson.
  • there's a reasonable chance that the generated plugin doesn't coexist peacefully with any other plugins, generated then subsequently modified and built with npm run build. see @wordpress/scripts: React.createElement type is invalid when two single block plugins are built using npm run build #24321

  • Question: Should package.log.json be listed in .gitignore?

  • The throw new error logic, invoked when index.asset.php is not present, is a very poor user experience. A Fatal error from which an end user may have great difficulty recovering.

Screenshots
image

Desktop (please complete the following information):

  • OS: [e.g. iOS] Windows
  • Browser [e.g. chrome, safari] Chrome
  • Version [e.g. 22] latest

Additional context
node version: v10.13.0
npm version: 6.14.5
from package.json

"devDependencies": {
		"@wordpress/scripts": "^12.1.1"
	},
@gziolo
Copy link
Member

gziolo commented Oct 14, 2020

Thank you for the detailed report, much appreciated.

Noting now that #24125 might address some of the issues mentioned. I plan to take a closer look in the upcoming days to confirm.

@gziolo
Copy link
Member

gziolo commented Oct 23, 2020

The text domain is oik-sb. It should be on-this-day, the same name as the plugin.

This is correct now.

the PHP code doesn't call wp_set_script_translations.

This is correct now.

there is no call to load_plugin_textdomain
the build commands do not include npm run makepot and npm run makejson.

Can you share more details about those issues and some pointers on how to fix them? It's something that doesn't exist in @wordpress/scripts. @ocean90 @swissspidy – you probably can help here better than I do.

Question: Should package.log.json be listed in .gitignore?

Did you mean package-lock.json maybe?

The throw new error logic, invoked when index.asset.php is not present, is a very poor user experience. A Fatal error from which an end user may have great difficulty recovering.

This is something that should never happen in production so I wouldn't consider it a user issue but a plugin distribution issue. Still, if you have a proposal on how to improve handling there, we can quickly patch it 👍

@gziolo gziolo added the [Status] In Progress Tracking issues with work in progress label Oct 23, 2020
@gziolo gziolo self-assigned this Oct 23, 2020
@swissspidy
Copy link
Member

there is no call to load_plugin_textdomain
the build commands do not include npm run makepot and npm run makejson.

For plugins distributed on WordPress.org this is not needed.

For other plugins it‘s a bit different.

@gziolo
Copy link
Member

gziolo commented Oct 29, 2020

Thanks @swissspidy for clarification. I think it confirms that the main issue reported is fixed for plugins distributed on WordPress.org which is the main goal of @wordpress/create-block.

There are still some open questions but they might deserve their own issues.

@gziolo gziolo closed this as completed Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress [Tool] Create Block /packages/create-block
Projects
None yet
Development

No branches or pull requests

4 participants