Functions with signatures more similar to those in WordPress

[johnbillion]

When switching from using wp_enqueue_script() and co to using this package's autoenqueue(), the difference in the function interfaces is a little jarring. I'd like to recommend the addition of two helper functions which:

• Use signatures a little more familiar to WordPress developers in order to make the transition easier
• Remove the need to specify an asset manifest path which is IMO an implementation detail
• Get around Support CSS-only entrypoints. #1 by silently rewriting a .css path to .js (which is working for theme stylesheets on the SW project)

function register_asset( string $handle, string $file, array $options = [] ) :? array {
	$file = str_replace( '.css', '.js', $file );
	$manifest = dirname( $file ) . '/build/asset-manifest.json';
	$options['handle'] = $handle;
	return autoregister( $manifest, basename( $file ), $options );
}

function enqueue_asset( string $handle, string $file, array $options = [] ) : void {
	$file = str_replace( '.css', '.js', $file );
	$manifest = dirname( $file ) . '/build/asset-manifest.json';
	$options['handle'] = $handle;
	autoenqueue( $manifest, basename( $file ), $options );
}

Usage looks like:

enqueue_asset(
	'theme-style',
	get_stylesheet_directory(). '/style.css',
	[
		'styles' => [ 'dashicons' ],
	]
);

Thoughts? I can work on a PR if we agree these would be useful additions.

[svandragt]

Strong agree, would love to see this, it's better at following the conventions of the WordPress community.

[kadamwhite]

I see the potential utility of this interface, but to me, fundamentally what we're doing here is work around how WordPress loads scripts -- I'm therefore hesitant to make magic methods that obscure and obstruct understanding of what's going on, based on a gut instinct that doing so is going to set developers up for increased confusion when they find that things don't work as expected when the dev server isn't running or a build errors.

I'm also a bit uncertain how you could reliably detect an asset-manifest based purely on an asset URI disk path; do you have ideas there?

this week I'm in the process of making a new interface that gets a bit away from the "magic" of our own autoenqueue process, and while it still takes a manifest-first approach it hopefully will move us a bit closer towards the interface you're requesting. Once that next release is out, I'd welcome further discussion here, or a PR to add helper functions to work the way you like.

[kadamwhite]

To clarify my concern above, the issue is almost entirely the removal of the manifest piece. Each of the past three projects I've worked on used a different file hierarchy within the build folder, and short of trying each parent directory in turn I can't think of a simple way we could have used the same logic to find the asset-manifest.json starting from every different asset in those projects.

The signatures I'm considering for the 0.4 version would look in other ways quite similar to what you're proposing, although they would maintain the focus on the manifest location, and the entry point name within that, because doing so lets us very easily handle cases like hashed filenames #2. This is what I have now:

	AssetLoader\enqueue_asset(
		manifest_file_path(),
		'propose-draft-date.js',
		[
			'handle'  => EDITOR_BUNDLE_HANDLE,
			'scripts' => [
				'wp-components',
				'wp-i18n',
				'wp-date',
				'wp-data',
				'wp-edit-post',
			],
		]
	);

which could be further broken out into

	Asset_Loader\enqueue_script(
		manifest_file_path(),
		'propose-draft-date.js', // Key within asset manifest
		EDITOR_BUNDLE_HANDLE,
		[
			'wp-components',
			'wp-i18n',
			'wp-date',
			'wp-data',
			'wp-edit-post',
		]
	);
	Asset_Loader\enqueue_style(
		manifest_file_path(),
		'frontend-styles.css', // Key within asset manifest
		'my-theme-styles',
		[]
	);

I feel relatively strongly that opening with the manifest and the expected manifest entry (slash, the generated filename) is the least error-prone solution, but would welcome your input if you disagree. If we did make script/style-specific methods, I'm also open to suggestions on whether enumerating the handle and dependencies separately, or as part of one options object, would be preferable.

[johnbillion]

In my mind the path to the asset manifest file is an implementation detail that a developer shouldn't be concerned about. As a developer onboarding onto a project I shouldn't even care that this file can exist.

If there are significant differences in the location of asset-manifest.json between projects then I see your point about its detection not being reliable, but that in itself seems like a problem to solve at the project level not at the level of each script or style enqueue.

Could there be a configuration constant or Webpack config that informs Asset Loader of the pattern to use when looking for asset-manifest.json?

[kadamwhite]

that in itself seems like a problem to solve at the project level

I completely agree on this point, but I believe that's something we should solve externally to this project. I've almost always regretted making functions more "magic," and my argument is that if we solve the project consistency problem, then setting up a new resource becomes a question of "do it like you see over here" and becomes as easy as the magic methods you're requesting would be.

Could there be a configuration constant or Webpack config that informs Asset Loader of the pattern to use when looking for asset-manifest.json?

This is the core disagreement between us, I think: I consider the asset-manifest.json to be the way Webpack signals to PHP what patterns and paths to use. I don't consider it an implementation detail that's irrelevant to a developer.

Even with a consistent set of project structures, I can imagine situations where I'd want to spit out a separate build elsewhere in the file tree. I'd need to specify a manifest or use a lower-level loader method to do that, and I feel the inconsistency of "why do we do it one way here, and another way there" is worse than having to make developers aware that there's a critical file which is necessary for their build to work correctly. Burying that file's existence behind internal path logic perpetuates Webpack's "black box" reputation, and that means it continues to be harder for the team at large to debug.

We're using the manifest in the manner that the developers of the Webpack plugin expect it to be used. As a developer onboarding to a project, I think it's a feature, not a bug, that I would need to understand how the manifest is used to connect my assets. If I don't understand that, then no amount of clever path inference is going to make things easier for me if I change something and my scripts stop loading. In that context I am confidant in the current API that foregrounds the manifest.

[fklein-lu]

We're using the manifest in the manner that the developers of the Webpack plugin expect it to be used.

This is at the core of this project. The Asset Loader is a bridge from Webpack to WordPress, and not the other way around. Therefore the function signature follows Webpack conventions, and not WordPress conventions.

Even with a consistent set of project structures, I can imagine situations where I'd want to spit out a separate build elsewhere in the file tree.

This is a consistent need when streamlining builds into a consistent Webpack setup, and therefore this flexibility is welcome.

[svandragt]

As a developer onboarding to a project, I think it's a feature, not a bug, that I would need to understand how the manifest is used to connect my assets.

Conceptually the manifest is connected to the asset once per unit of code (plugin/theme/project) not once per use unless you're planning to use a 1:1 ratio of assets to manifests, which seems unlikely. Therefore it makes more sense to me as a mental model to consider it as part of the bootstrap. It doesn't feel like a decision that should be revisited every time an asset is enqueued. So having the developer have this overhead reduced seems like a win.

[johnbillion]

@kadamwhite @fklein-lu Great points about the fact that this is a bridge from Webpack to WordPress and not the other way around and should therefore focus on Webpack conventions. I think @svandragt 's point is spot on, that the asset-manifest.json does have importance but in my mind isn't something that should be considered at the point where each individual asset gets enqueued. AFAICT it is a per-plugin/theme configuration and there would be no reason for it to vary between assets. Is this correct?

[kadamwhite]

AFAICT it is a per-plugin/theme configuration and there would be no reason for it to vary between assets. Is this correct?

Yup, this is correct. One potential workaround would be for a plugin or theme to define a theme or plugin-level method that would "fill in" the manifest location,

/**
 * Register and immediately enqueue a particular asset within a manifest.
 *
 * @param string $target_asset  Asset to retrieve within the specified manifest.
 * @param array  $options {
 *     @type string $handle       Handle to use when enqueuing the asset. Optional.
 *     @type array  $dependencies Script or Style dependencies. Optional.
 * }
 */
function enqueue_asset( string $target_asset, array $options = [] ) : void {
    Asset_Loader\enqueue_asset( 'manifest path here', $target_asset, $options );
}

This method could then be used anywhere in the theme or plugin that an asset is needed. Would this be an acceptable compromise? I briefly considered some sort of with_manifest factory inside this library that would use a closure to store the manifest string and return a helper, but it feels more direct to implement it on the plugin or theme side using the code above.

[kadamwhite]

After a chat with @johnbillion today, we're provisionally going to move forward with the recommendation in my comment above, and account for that in the documentation that needs to be written for this project (similar to the docs site for webpack helpers. I'll leave this issue open until we have clear place we can point to which explains the intended usage.