Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEWP docs and implementation mismatch/confusion in regards of assets file name #49872

Open
tomalec opened this issue Apr 17, 2023 · 2 comments · May be fixed by #45948
Open

DEWP docs and implementation mismatch/confusion in regards of assets file name #49872

tomalec opened this issue Apr 17, 2023 · 2 comments · May be fixed by #45948
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Tool] Dependency Extraction Webpack Plugin /packages/dependency-extraction-webpack-plugin [Type] Developer Documentation Documentation for developers

Comments

@tomalec
Copy link
Contributor

tomalec commented Apr 17, 2023

Description

The Dependency Extraction Webpack Plugin docs states:

// Source file entrypoint.js
// […]
// Webpack will produce the output output/entrypoint.js
// […]
// Webpack will also produce output/entrypoint.asset.php declaring script dependencies

Therefore, when I have entry: { 'analytics': '…' }, I'd expect the assets file name to be analytics.asset.php.

However, it's effectively different, as per implmentation it takes explicitly output's file name (assetFilename = compilation.getPath( outputFilename). Which can be completely different.

Step-by-step reproduction instructions

  1. Create a bundle that uses DEWP (and default WordPress webpack config)
{
	...require( '@wordpress/scripts/config/webpack.config' ),
	entry: {
		'entrypoint': './path/to/some/index.js',
	},
	output: {
		filename: 'bunny-plugin-[name].min.js',
		path: './output/',
	},
  1. Run the build.
  2. 🔴 Check that assets file is output/bunny-plugin-entrypoint.min.asset.php not output/entrypoint.asset.php.

So I'd rather describe it as output/filename.asset.php

Screenshots, screen recording, code snippet

No response

Environment info

n/a - I'm using only @wordpress/scripts and @wordpress/dependency-extraction-webpack-plugin

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@gziolo gziolo added [Type] Developer Documentation Documentation for developers Good First Issue An issue that's suitable for someone looking to contribute for the first time [Tool] Dependency Extraction Webpack Plugin /packages/dependency-extraction-webpack-plugin labels Apr 17, 2023
@mburridge mburridge self-assigned this May 25, 2023
@mburridge
Copy link
Contributor

@tomalec thanks for raising this issue. As far as I can determine the documentation is correct and by default the output file generated behind the scenes will indeed be entrypoint.asset.php.

You are correct that explicitly defining an output filename will over-ride the default configuration, however, this is fairly advanced webpack usage, so I'm not sure we need an update to the documentation on this.

Happy to hear your thoughts on this.

@tomalec
Copy link
Contributor Author

tomalec commented Aug 25, 2023

Thank you for the answer @mburridge .

Personally, I do not consider output an "advanced webpack usage". Output different than entry point is given in the very first example of webpack config in their docs: https://webpack.js.org/configuration/configuration-languages/#typescript, https://webpack.js.org/concepts/configuration/#introductory-configuration

As far as I can determine the documentation is correct and by default the output file generated behind the scenes will indeed be entrypoint.asset.php.

It is correct, but only for a very specific config:

const path = require('path');

module.exports = {
  ...require('@wordpress/scripts/config/webpack.config'),
  entry: {
    entrypoint: './entrypoint.js',
  },
  output: {
    path: path.resolve(__dirname, 'output'),
  },
};

And we do not mention this or any similar config in the sample. And I would not call the above a "default".

Webpack's defaults

The default, Webpack's output-dir is not output but dist, and the output filename is main.js. Take the really minimalistic setup, with entrypoint.js as a source, and stick to all the defaults:

module.exports = {
  entry: './entrypoint.js',
};

The result is dist/main.js

@wordpress/scripts defaults

With minimalistic @wordpress/scripts webpack setup:

module.exports = {
	...require( '@wordpress/scripts/config/webpack.config' ),
  entry: './entrypoint.js',
};

The result is:

/
├── build/
│   ├── main.asset.php
│   ├── main.js
│   └── main.js.map
├── entrypoint.js
└── webpack.config.js

There is no output/entrypoint.asset.php in the above "default" configs.

See live examples at https://stackblitz.com/edit/github-rigj39?file=webpack.config.js


Therefore, I think of three approaches:

  1. If we're about to stick to defaults, we could write it as:
// Source file my-file.js
// […]
// Webpack will produce the output build/main.js
// […]
// Webpack will also produce build/main.asset.php declaring script dependencies
  1. Do not use default values but just descriptive names, referring webpack's configuration
// Source file entry.js
// […]
// Webpack will produce the output output/path/filename.js
// […]
// Webpack will also produce output/path/filename.assets.php declaring script dependencies
  1. Provide a sample config, and use the actual results from that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time [Tool] Dependency Extraction Webpack Plugin /packages/dependency-extraction-webpack-plugin [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants