Skip to content

Conversation

@tomalec
Copy link
Contributor

@tomalec tomalec commented Sep 24, 2021

Description

This PR adds a new option for plugin config externalizedReportFile, to report all dependencies exported for the current project.
I need it as I try to build a tool that helps to inspect, analyze and manage the versions being used for local unit tests, and that runs with WordPress.
To compare the versions used locally, with the versions used in the range of supported WP builds.


edit:
This PR tries to address problem 5. from #35630

As a plugin developer, I don't even know what dependencies are extracted, as the list is maintained in the package repo, and not reported while bundling.

Thanks to this PR a developer would at least know which of their local dependencies are extracted and which are not. So for which packages do they need to anticipate different versions.


How has this been tested?

I run this code against
https://github.com/woocommerce/google-listings-and-ads repo's webpack config with and without the new option.

  1. git clone https://github.com/woocommerce/google-listings-and-ads.git
    cd google-listings-and-ads
    npm install
    
  2. Replace node_modules/@wordpress/dependency-extraction-webpack-plugin with the changed one from this PR packages/dependency-extraction-webpack-plugin.

Without the new option

  1. Use the repo's Webpack config as is https://github.com/woocommerce/google-listings-and-ads/blob/4e69b169953096774dd0d957ba7a4fcecbbf2a89/webpack.config.js#L79-L83
     	new DependencyExtractionWebpackPlugin( {
     		injectPolyfill: true,
     		requestToExternal,
     		requestToHandle,
     	} ),
  2. npm run build
  3. Check that the /js/build/index.js bundle is created as before.

With the new option

  1. Add the reportExternalized option to the /webpack.config.js#L79-L83:
     	new DependencyExtractionWebpackPlugin( {
     		injectPolyfill: true,
     		requestToExternal,
     		requestToHandle,
     		externalizedReportFile: 'externalized.json',
     	} ),
  2. npm run build
  3. Check that the /js/build/index.js bundle is created as before.
  4. Check the /js/build/externalized.js is created and contains:
    ["@wordpress/i18n","@wordpress/hooks","@woocommerce/navigation","@woocommerce/wc-admin-settings","@wordpress/element","@woocommerce/components","@wordpress/api-fetch","@wordpress/data","@wordpress/deprecated","@babel/runtime/regenerator","@wordpress/date","lodash","react","@wordpress/html-entities","moment","@wordpress/is-shallow-equal","@wordpress/keycodes","@wordpress/a11y","@wordpress/priority-queue","@wordpress/dom","react-dom","@wordpress/warning","@wordpress/rich-text","@wordpress/data-controls","@wordpress/url"]

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested. - tested manually, I was not able to run tests for this package only, and npm install for entire repo fails on my machine
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • [n/a] I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • [n/a] I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@tomalec tomalec requested a review from gziolo as a code owner September 24, 2021 09:31
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Sep 24, 2021
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @tomalec! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@gziolo
Copy link
Member

gziolo commented Oct 4, 2021

Thank you for opening this Pull Request.

I need it as I try to build a tool that helps to inspect, analyze and manage the versions being used for local unit tests, and that runs with WordPress (See articles: pb0Spc-1LK-p2, p9oQ9f-zJ-p2).

How people can access those articles? It's hard to understand how the changes proposed would benefit the WordPress project and the community without this context.

The build process in Gutenberg and WordPress core replaces all WP dependencies with wp.* globals. In practice, the versions of WP packages or core libraries like lodash or react used in the development will always differ from those in WordPress because they depend on the version of WordPress core and whether a Gutenberg plugin is installed.

@gziolo gziolo added [Tool] Dependency Extraction Webpack Plugin /packages/dependency-extraction-webpack-plugin [Type] New API New API to be used by plugin developers or package users. labels Oct 5, 2021
@tomalec
Copy link
Contributor Author

tomalec commented Oct 10, 2021

How people can access those articles? It's hard to understand how the changes proposed would benefit the WordPress project and the community without this context.

I created an issue in this repo #35630
I hope it gives enough context and rationale to investigate this problem, and partial solution.

This PR tries to address problem 5. stated there

As a plugin developer, I don't even know what dependencies are extracted, as the list is maintained in the package repo, and not reported while bundling.
That may result in unexpected behavior and error. Checking that again requires even more manual effort. As it requires digging into the source code of DEWP at a given version and manually comparing packages list.


The build process in Gutenberg and WordPress core replaces all WP dependencies with wp.* globals. In practice, the versions of WP packages or core libraries like lodash or react used in the development will always differ from those in WordPress because they depend on the version of WordPress core and whether a Gutenberg plugin is installed.

That's exactly the DevX problem I'm trying to address here.


Thanks to this PR a developer would at least know which of their local dependencies are extracted and which are not. So for which packages do they need to anticipate different versions.

Copy link
Member

@jsnajdr jsnajdr left a comment

Choose a reason for hiding this comment

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

Works well for me 👍

@gziolo
Copy link
Member

gziolo commented Oct 12, 2021

I run this code against
https://github.com/woocommerce/google-listings-and-ads repo's webpack config with and without the new option.

What do I have to provide to enable this option? Can you share a full example?

@jsnajdr
Copy link
Member

jsnajdr commented Oct 12, 2021

What do I have to provide to enable this option? Can you share a full example?

I tested this new option by modifying the tools/webpack/shared.js config file and running npm run build. A report.json file was created.

@gziolo
Copy link
Member

gziolo commented Oct 14, 2021

Thinking a bit more about it, if you want to see a full list of externalized dependencies you can enable combineAssets flag and it will report all of them in a single file grouped by chunks. It is also what WordPress core uses:

https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/assets/script-loader-packages.php

@tomalec
Copy link
Contributor Author

tomalec commented Oct 14, 2021

What do I have to provide to enable this option? Can you share a full example?

I added more elaborate steps to reproduce in the OP #35106 (comment)

Is that clearer now?

@tomalec
Copy link
Contributor Author

tomalec commented Oct 14, 2021

Thinking a bit more about it, if you want to see a full list of externalized dependencies you can enable combineAssets flag and it will report all of them in a single file grouped by chunks. It is also what WordPress core uses:

WordPress/wordpress-develop@master/src/wp-includes/assets/script-loader-packages.php

Interesting. I didn't think combineAssets flag could serve to report dependency.
After reading the API docs:

By default, one asset file is created for each entry point. When this flag is set to true, all information about assets is combined into a single assets.(json|php) file generated in the output directory.

I thought it is to change the behavior of generating assets separately for each entry point to generating a single file. So to change the way it actually exposes the assets to be used on runtime.

Also, for

new DependencyExtractionWebpackPlugin( {
	  combineAssets: false,
	  outputFormat: 'json',
} );

it generates this list of internal identifiers

{
	"index.js": {
		"dependencies": [
			"lodash",
			"moment",
			"react",
			"react-dom",
			"wc-components",
			"wc-navigation",
			"wc-settings",
			"wp-a11y",
			"wp-api-fetch",
			"wp-data",
			"wp-data-controls",
			"wp-date",
			"wp-deprecated",
			"wp-dom",
			"wp-element",
			"wp-hooks",
			"wp-html-entities",
			"wp-i18n",
			"wp-is-shallow-equal",
			"wp-keycodes",
			"wp-polyfill",
			"wp-priority-queue",
			"wp-rich-text",
			"wp-url",
			"wp-warning"
		],
	// …

instead of npm package names module specifiers used in the local (plugin's) codebase

[
	"@wordpress/i18n",
	"@wordpress/hooks",
	"@woocommerce/wc-admin-settings",
	"@woocommerce/navigation",
	"@wordpress/element",
	"@woocommerce/components",
	"@wordpress/data-controls",
	"@wordpress/data",
	"@wordpress/api-fetch",
	"@babel/runtime/regenerator",
	"@wordpress/url",
	"@wordpress/date",
	"lodash",
	"moment",
	"@wordpress/deprecated",
	"@wordpress/a11y",
	"@wordpress/html-entities",
	"react",
	"@wordpress/keycodes",
	"@wordpress/priority-queue",
	"react-dom",
	"@wordpress/dom",
	"@wordpress/is-shallow-equal",
	"@wordpress/rich-text",
	"@wordpress/warning"
]

But that's pretty close, do you think we can somewhat merge those two together?
Like serializing the actual Map< externalizedDependency, scriptDependency >?

@jsnajdr
Copy link
Member

jsnajdr commented Oct 28, 2021

if you want to see a full list of externalized dependencies you can enable combineAssets flag

Seems to me that the purposes of combineAssets and externalizedReportFile are quite different and it's not advisable to mix them:

  • combineAssets specifies how that *.asset.php files should look like, and these files are part of the published artifact. WordPress PHP code that loads the scripts depends on them.
  • externalizedReportFile is not supposed to be published -- it's an intermediate report/log/stats file that's supposed to be an input to some next step in the build pipeline. Or helps the developer with debugging their dependencies.

@tomalec
Copy link
Contributor Author

tomalec commented Nov 15, 2021

@gziolo What's your take on the above comments?

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I think we should focus on bringing ES Modules and import maps to WordPress core to eventually make this plugin obsolete. It’s going to be a long process so if you think it’s helpful to add this feature then let’s land it.

@gziolo
Copy link
Member

gziolo commented Nov 19, 2021

One of the Continues Integration jobs fails even on subsequent re-tries: https://github.com/WordPress/gutenberg/runs/4261297083?check_suite_focus=true

Can you check if rebasing with trunk resolves it? I doubt there's anything wrong with this branch.

tomalec and others added 5 commits November 19, 2021 18:06
to report all dependencies extracted for the burrent build.
Use the default filename - `'externalized-dependencies.json'` for `true`.
Addresses WordPress#35106 (review)
@tomalec
Copy link
Contributor Author

tomalec commented Nov 19, 2021

Can you check if rebasing with trunk resolves it? I doubt there's anything wrong with this branch.

Thanks, @gziolo . Rebased, everything is green now.

@gziolo gziolo merged commit 6ba7c24 into WordPress:trunk Nov 19, 2021
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 19, 2021
tomalec added a commit to woocommerce/google-listings-and-ads that referenced this pull request Nov 23, 2021
Report externaized dependencies to `/.externalized.json`.
Help to inspect how wide range of granular dependencies we need to cover.

Use `trunk` version of `@wordpress/dependency-extraction-webpack-plugin` to be able to use unreleased
WordPress/gutenberg#35106
tomalec added a commit to woocommerce/google-listings-and-ads that referenced this pull request Nov 23, 2021
Report externa;ized dependencies to `/.externalized.json`.
Help to inspect how wide is the range of granular dependencies we need to anticipate.

Use `trunk` version of `@wordpress/dependency-extraction-webpack-plugin` to be able to use unreleased
WordPress/gutenberg#35106
@tomalec tomalec deleted the add/dewp-report branch December 9, 2021 18:29
@tomalec
Copy link
Contributor Author

tomalec commented Dec 9, 2021

@gziolo is there any date when this (and two other PRs) could be released to npm? - so it could be easily used outside

@gziolo
Copy link
Member

gziolo commented Dec 13, 2021

See: https://github.com/WordPress/gutenberg/blob/trunk/docs/contributors/code/release.md#synchronizing-wordpress-trunk

For each Gutenberg plugin release, WordPress trunk should be synchronized. Note that the WordPress trunk branch can be closed or in "feature-freeze" mode. Usually, this happens between the first beta and the first RC of the WordPress release cycle. During this period, the Gutenberg plugin releases should not be synchronized with WordPress Core.

We are in the process of releasing the next major WordPress version (5.9) so we don't do regular npm publishing every two weeks.

I plan to run the development release with the next dist-tag later this week in case someone wants to use the most recent version earlier.

@tomalec
Copy link
Contributor Author

tomalec commented Dec 14, 2021

Thank you for the info!

BTW, I made a small improvement for this feature in #37377, maybe it could get into that release as well?

@gziolo
Copy link
Member

gziolo commented Dec 20, 2021

It's now available for testing on npm with:

npm install --save-dev @wordpress/dependency-extraction-webpack-plugin@next

The stable version should be available in the second half of January.

@tomalec
Copy link
Contributor Author

tomalec commented Dec 20, 2021

Awesome, thanks for the heads-up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Tool] Dependency Extraction Webpack Plugin /packages/dependency-extraction-webpack-plugin [Type] New API New API to be used by plugin developers or package users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants