Refine asset handling to better reflect modern block development

[fabiankaegy]

Description of the Change

Refine the asset handling of scripts and styles defined in block.json files. For one assets that get defined via the script, editorScript, viewScript, style, and editorStyle keys in a block.json file now get automatically added as entry points. No manual configuration is needed.

It also copies the block.json file to the dist directory so the file paths can really be relative. The block.json file in the dist folder is the one that should be registered in WordPress. (a follow-up PR in the wp-scaffold is required to make this work.)

Closes #184 & #130

Alternate Designs

Possible Drawbacks

Verification Process

Checklist:

• I have read the CONTRIBUTING document.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my change.
• All new and existing tests passed.

Changelog Entry

Credits

Props @

[github-actions]

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	2	0
Ignored	0	N/A

• Result: ✅ success

• Annotations: 2 total

  ---

[warning] no-console

disallow the use of console

• projects/10up-theme/includes/blocks/example-block/script.js Line 4 - Unexpected console statement.
• projects/10up-theme/includes/blocks/example-block/view.js Line 4 - Unexpected console statement.

---

^{Report generated by eslint-plus-action}

[fabiankaegy]

One of the problems that has not yet been solved is getting HMR of the CSS files to work with this setup. I am currently setting up the entry point based on the block.json style & editorStyle keys.

@nicholasio do you have any idea how we can get this to work whilst still respecting the filenames of the assets that are getting imported into the JS files? (currently I am not importing them at all)

[nicholasio]

One of the problems that has not yet been solved is getting HMR of the CSS files to work with this setup. I am currently setting up the entry point based on the block.json style & editorStyle keys.

@nicholasio do you have any idea how we can get this to work whilst still respecting the filenames of the assets that are getting imported into the JS files? (currently I am not importing them at all)

I think the easiest way to enable HMR of the CSS files would be to create a webpack plugin that would transform the block source file to include something like this:

// editorScript
if ( __DEV__ ) {
  import('./path-to-source-editor-css-taken-from-block.json');
}

// script
if ( __DEV__ ) {
  import('./path-to-source-css-taken-from-block.json');
}

[fabiankaegy]

I had a quick chat with @Antonio-Laguna about this and we wanted to write up some specs for what this webpack plugin would actually need to do.

With this change, the webpack entry points for the block-specific assets get automatically registered from the script, editorScript, viewScript, style, and editorStyle keys in every block.json file.

This means that you no longer have to manually go into your package.json to register each of the entry points manually.
Because of this, we are not importing the CSS files (style & editorStyle) into any js file because they are native entry points. Therefore the stylesheets don't get included in the Hot Module Replacement.

This is what we want to fix.

I'm not currently familiar enough with how exactly HMR works but at a very high level, we need to make sure that the CSS files get imported to a JS entry point when the watch mode is active.

Open Questions:

1. Where do we actually need to import the CSS files in order for them to be picked up by HMR? Does the style stylesheet need to be imported into a js file that is enqueued on the frontend? Or can it be imported anywhere no matter whether the script is loaded?
2. How does HMR understand where the actual CSS file is enqueued? / Do we need to enqueue a separate file in the watch mode in order for it to work?

[fabiankaegy]

@nicholasio Any idea why the tests started failing after I merged the develop tranch back into this branch? The errors shown in the tests don't seem to be related to the changes I introduced.

[changeset-bot]

🦋 Changeset detected

Latest commit: 940821a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
10up-toolkit	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[fabiankaegy]

We discussed this PR in out internal 10up-toolkit council call today. We talked about the fact hat given that we are using add_editor_styles function from WordPress to enqueue our editor styles we don't get HRM for our editor styles. And that this update is not intended to change anything about our best practice approach of writing the actual CSS of the blocks in the main frontend.css bundle.

So we are not actually looking any HMR which unblocks this PR.

The next steps are for me @fabiankaegy to rebase this PR and update all the notes @nicholasio had made. Additionally we will be doing some more testing but hopefully this PR should be ready to get merged in the next few weeks :)

[fabiankaegy]

@nicholasio do you have any idea why there are so many changes in the package-lock.json file? 🤔

I am using nvm to pull in the configured node version (using node v16.14.2 (npm v8.5.0))

[fabiankaegy]

@nicholasio @Antonio-Laguna I have just updated this PR to ship the new block asset handling behind a feature flag. After doing some testing it does alter some things in your setup that you need to account for which is why I think we would need to ship this behind a feature flag.

My approach would be that after this gets merged I would create a follow-up PR for the 10up wp-scaffold to enable the feature flag so that new projects get this behavior by default. Any other projects that want to use it can feel free to enable it but don't need to and therefore it isn't a breaking change.

Would love to hear whether you have any thoughts on that approach or ideas how else this could be solved :)

[Antonio-Laguna]

@fabiankaegy I think this is good as a feature flag! This way it does not need to be a breaking change. I think it makes sense to document the differences between the flags, what you get/what you don't get so developers can make an informed decision here.

I think the lock doesn't get frequently updated, any publishing should update the lock too since changes in internal versions do trigger changes within package-lock.json. I could be wrong and this is already happening but feels odd to get this many changes!

[nicholasio]

LGTM.

Made a small change to enable the flag on the test project. Feel free to merge if all looks good to you.