Checking chunk path for block decission

[Antonio-Laguna]

#264

Related Issue/RFC: #

Description of the Change

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.
• I have added a changeset to my PR. See CONTRIBUTING document for instructions

[changeset-bot]

🦋 Changeset detected

Latest commit: ff2e5da

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	Patch

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

[Antonio-Laguna]

I was about to give up on this since WebPack's docs are terrible and console.log gave me little information so I had to investigate the prototype and source codes to find a way to extract the chunk's path.

I'm tempted to remove the check by name and just rely on directory belonging but I fear it might break.

I've assigned it for double checking (though there's not much code there changed) but also it'd be good if you could manually change this within a couple of project's node_module/10up-toolkit/config/webpack/plugins.js and copy this fix. Ideally, everything should behave normally but you no longer need a -block- prefix anywhere.

[roseg43]

@Antonio-Laguna Thanks for putting in a PR for this so quickly! I'll take a look early this week and confirm that this is working as expected

[roseg43]

@Antonio-Laguna I tested this manually on a project this morning and while I'm not seeing any errors when running 10up-toolkit build, I am getting the following error when attempting to use the watch command:

Module.entryModule: Multiple entry modules are not supported by the deprecated API (Use the new ChunkGroup API)
Error: Module.entryModule: Multiple entry modules are not supported by the deprecated API (Use the new ChunkGroup API)
    at Chunk.get entryModule [as entryModule] (/Users/gabrielrose/Local Sites/<sitename>/app/public/wp-content/plugins/custom-block-library/node_modules/webpack/lib/Chunk.js:122:10)
    at filename (/Users/gabrielrose/Local Sites/<sitename>/app/public/wp-content/plugins/custom-block-library/node_modules/10up-toolkit/config/webpack/plugins.js:106:36)

[Antonio-Laguna]

@roseg43 that's odd. I have tried it on the local theme and it works perfectly. I'm surprised it would break like that since this does not add anything new but will match more cases rather than just being -block in the name. The rest is identical.

@fabiankaegy could you double-check this to see if the output is as expected?

[Antonio-Laguna]

Going to tackle #277 here as this touches the same area

[nicholasio]

I think this has the potential to cause issues... and the reason is that things can get really confusing.....

If useBlocksAsset is being used then filenames is overridden here and that makes things work mostly fine. (now that I see where filenames variable is being changed I realize that is not the best place as it causes side-effects elsewhere - it took me a while to figure out who was mutating that variable)

But when useBlocksAsset is not set to true which is the default behavior then filenames.blockCSS is blocks/[name]/editor.css. Then here's what happens:

• The CSS is generated at blocks/[name]/editor.css
• Any CSS file imported from the JS of the block would also be generated at blocks/[name]/editor.css (currently this ends up being generated at css/[name].css - which is the value of filenames.css.

I actually think this is the issue @roseg43 is running into #267 (comment) (@roseg43 are you using useBlocksAssets?)

I think the following code would be backwards compatible:

// this returns what you want but would be undefined for CSS 
// that comes from non-css files (e.g CSS imported via JS)
const fullPath = options.chunk.entryModule.resource; 
if (!fullPath) {
 return filenames.css;
}

I did just a few tests but I'm thinking we should document the current behavior just to be extra sure of how it works and make sure the changes here do not break the current behavior.

[nicholasio]

The local theme we use for testing things has useBlocksAssets enabled so that's prob why @Antonio-Laguna wasn't running into this issue.

[Antonio-Laguna]

🙇 thanks a lot @nicholasio I would have sworn that I had tested this in both scenarios but you're more than right! I've updated this

[roseg43]

Ah yep, I definitely didn't have it enabled!