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.

Sign up for GitHub

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

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

Merged
merged 6 commits into from
Jun 26, 2019
Merged

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

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d49efd6
Add support for watching block.json files when running `npm run dev`
talldan Jun 13, 2019
6f3cc66
Revert "Add support for watching block.json files when running `npm r…
talldan Jun 20, 2019
40360e6
Handle block.json transform in the build.js script
talldan Jun 20, 2019
84acb60
Add jsdoc comment
talldan Jun 26, 2019
ae18434
Fix path matching on Windows
talldan Jun 26, 2019
780350c
Simplify block.json build transform
talldan Jun 26, 2019
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
34 changes: 33 additions & 1 deletion bin/packages/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,36 @@ function createStyleEntryTransform() {
} );
}

function createBlockJsonEntryTransform() {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@talldan talldan Jun 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the separate PR: #16317

const blocks = new Set;

return new Transform( {
objectMode: true,
async transform( file, encoding, callback ) {
const matches = /block-library\/src\/(.*)\/block.json$/.exec( file );
Copy link
Member

Choose a reason for hiding this comment

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

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 ))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

const blockName = matches ? matches[ 1 ] : undefined;

// Only block.json files in the block-library folder are subject to this transform.
if ( ! blockName ) {
this.push( file );
callback();
return;
}

// Only operate once per block, assuming entries are common.
if ( blockName && blocks.has( blockName ) ) {
callback();
return;
}

blocks.add( blockName );
const entries = await glob( path.resolve( PACKAGES_DIR, 'block-library/src/', blockName, 'index.js' ) );
entries.forEach( ( entry ) => this.push( entry ) );
callback();
},
} );
}

let onFileComplete = () => {};

let stream;
Expand All @@ -72,7 +102,9 @@ if ( files.length ) {
stream = new Readable( { encoding: 'utf8' } );
files.forEach( ( file ) => stream.push( file ) );
stream.push( null );
stream = stream.pipe( createStyleEntryTransform() );
stream = stream
.pipe( createStyleEntryTransform() )
.pipe( createBlockJsonEntryTransform() );
} else {
const bar = new ProgressBar( 'Build Progress: [:bar] :percent', {
width: 30,
Expand Down