Update plugin building #1008
Update plugin building #1008
Conversation
thelovekesh
added
[Type] Enhancement
webpack.config.js
Outdated
|
|
||
| const from = path.resolve( __dirname ); | ||
| const to = path.resolve( __dirname, 'build' ); | ||
| const pluginDistIncludes = [ |
There was a problem hiding this comment.
Isn't this list essentially the inverse of the current .gitattributes? Since this is an allowlist it will have to be manually updated. But then there is also a pluginDistIgnores which is essentially the same as the .gitattributes? Wouldn't this be simpler to just rather go with .distignore instead of the .gitattributes?
There was a problem hiding this comment.
Humm, I agree with the negation thing since it's replicating the behavior of .gitattributes. TBH I wasn't much aware of .distignore but after exploring it I understand it's just "another" ignore file but creating distribution builds.
There was a problem hiding this comment.
Since CopyWebpackPlugin uses globby which provides support for such ignore files, I have switched it to use .distignore which is pretty much neat.
There was a problem hiding this comment.
I think the scope of this PR needs to be pulled back because this branch is still working with modules and without the standalone plugins. In the feature/modules-to-plugins branch which will hopefully soon get merged, it has a build script for plugins already:
Line 26 in 36fe0dc
See the build-plugins command.
But then there's another PR #1000 which is branched off of that which goes further for supporting standalone plugins.
So I think all that should be done in the immediate term is to switch from .gitattributes to .distignore. But we should perhaps wait altogether until we we finally get the feature/modules-to-plugins branch merged into trunk, as otherwise it's likely there will be re-work needed or merge conflicts.
IMO instead of developing CLI scripts, which frequently need to be modified for new use cases, we should utilize a tool like Webpack, which is ideally equipped and made to handle such use cases. Also, it gives us the flexibility to consume WP Scripts configurations which includes better presets for such cases. |
|
Pulling this conversation for bundling plugins here. cc @felixarntz |
There was a problem hiding this comment.
@thelovekesh I am unsure what is the purpose of this PR, it looks like it is mixing concerns a bit.
I know it started as a PR for #897, which makes sense given it's targeting the feature/image-loading-optimization branch.
I don't think we should use this PR as a solution for the problem of ignoring development files and assets for the standalone plugins. That should be in its own PR against trunk. If you already have something in mind that can fix that problem, it would be great if you could open a new PR against trunk so we can consider it separately. This work shouldn't be coupled to a specific module.
| "build": "wp-scripts build", | ||
| "build-plugin": "wp-scripts build --env build=plugin", | ||
| "build-plugins": "./bin/plugin/cli.js build-plugins", |
There was a problem hiding this comment.
This is confusing to me. What is the purpose of build-plugin and build? We already have build-plugins which is for the modules to be built into plugins (which can soon be removed). Do we really need build-plugin or can it be combined somehow? If not, let's use a different name that clarifies what this is for.
|
Yes @felixarntz. This PR is confusing from the global point of view since this was originally authored as a POC for the ILO plugin. Hence, I will raise a PR to solve this on the trunk and later ILO can rebase with trunk and thus it will be solved for all the WIP and upcoming plugins. cc @westonruter |
|
@thelovekesh Sounds great, thanks! |
Summary
Fixes #893
Relevant technical choices
Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.