Add support for watching block.json files when running npm run dev

[talldan]

Description

Fixes #16104

This is quite a simplistic fix, but gets the job done!

The lowdown is that currently, block.json content is inlined into the index.js file its imported from when babel builds the files. The watch task currently doesn't work, firstly as the build-worker file had no task specified for building a json file. Secondly, such a task would have to rebuild the index.js relative to the block.json so that the inlined json is replaced when rebuilding.

This PR adds a new task that checks whether the file is a block.json file, and if so rebuilds the relative index.js file. It'll only handle block.json files in the block-library package, but helps solve a problem for most devs working on core blocks.

How has this been tested?

1. Run npm run dev
2. Load the editor and add an empty paragraph block, save the post
3. In the paragraph block's block.json, change the content attributes default to something else like "Hello, world!"
4. Observe in the console that a webpack rebuild is triggered
5. Reload the editor and click on the paragraph block you added earlier

On this branch:
The paragraph block contains the default defined in step 3, demonstrating that the watch task worked.

When running npm run dev against master:
The paragraph block isn't updated with its new default

Types of changes

Bug fix (non-breaking change which fixes an issue)

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.

[gziolo]

I can confirm that it solves the issue 🎉

I'm aware it isn't perfect but this should cover all the use cases we have. I don't see any immediate plans to use JSON files for anything but block.json files.

[aduth]

This seems to be a similar problem to one we encountered previously with the stylesheets (#15920), and as such I'd prefer to see a consistent approach used amongst the two of them. I'm not sure how far down the path of abstraction we want to go, but to me these appear to fall under a common behavior of a normalization process, where a file may serve as proxy to trigger another to be built instead. In #15920, the reason I had chosen to implement this in build.js and not in build-worker.js is to reserve the worker as minimally executing the computationally-intensive task (the transformation), and for all other filesystem management to occur in build.js (over time becoming to be clear to be a requirement, as evidenced by findings in #15967 (comment)).

[gziolo]

In #15920, the reason I had chosen to implement this in build.js and not in build-worker.js is to reserve the worker as minimally executing the computationally-intensive task (the transformation), and for all other filesystem management to occur in build.js (over time becoming to be clear to be a requirement, as evidenced by findings in #15967 (comment)).

I see what you mean after checking source code:

gutenberg/bin/packages/build.js

Lines 39 to 65 in d49efd6

	function createStyleEntryTransform() {
	const packages = new Set;
	return new Transform( {
	objectMode: true,
	async transform( file, encoding, callback ) {
	// Only stylesheets are subject to this transform.
	if ( path.extname( file ) !== '.scss' ) {
	this.push( file );
	callback();
	return;
	}
	// Only operate once per package, assuming entries are common.
	const packageName = getPackageName( file );
	if ( packages.has( packageName ) ) {
	callback();
	return;
	}
	packages.add( packageName );
	const entries = await glob( path.resolve( PACKAGES_DIR, packageName, 'src/*.scss' ) );
	entries.forEach( ( entry ) => this.push( entry ) );
	callback();
	},
	} );
	}

I wasn't aware it was architected this way. Now, I see it, it makes perfect sense to move it to build.js.

[gziolo]

@talldan do you plan to move logic according to what @aduth suggested? This error is really annoying when you actively work on adding new attributes to the block definition :)

[talldan]

@gziolo Yep, I'll take a look at that when I get a chance.

No longer relevant

[talldan]

This is ready for review again—reimplemented using the transform, similar to how scss stylesheets are handled.

[gziolo]

I tested again, it still works properly :)

[gziolo]

@aduth - would you mind doing a sanity check before we merge?

[aduth]

I'd be curious if this works in Windows, where file may have a different slash.

See also: https://nodejs.org/api/path.html#path_path_sep

I wonder instead if we:

• ... aren't too specific about whether the file is located, just that it's block.json (path.basename( file ) === 'block.json')
• ... consider the block name as the directory name in which the file is located (path.dirname( file ))

[talldan]

Good catch, I've improved the regex to handle both back and forward slash as the directory separator. I think that will cover all cases.

[aduth]

I realize we might be aiming for the most direct solution, but:

• There's a huge amount of code duplication between this and the above function, which could be generalized.
• We should document every function; in this case, explaining the purpose of the transform.

[talldan]

I've added some documentation, cheers for pointing that out.

I'll push up a separate PR for exploring generalization of these transforms. I had an initial attempt, but I feel like it could go through some iteration and I wouldn't want to hold up this main PR.

[talldan]

Here's the separate PR: #16317