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

Scripts: Breakage of multiple entry points of the same filename in separate folders #36497

Closed
stokesman opened this issue Nov 15, 2021 · 10 comments
Assignees
Labels
[Package] Scripts /packages/scripts [Type] Developer Documentation Documentation for developers
Projects

Comments

@stokesman
Copy link
Contributor

stokesman commented Nov 15, 2021

Description

The documentation for build and start demonstrates space separated paths for multiple entry points:

"build:custom": "wp-scripts build entry-one.js entry-two.js"

In a couple projects I used that approach but with each entry point in a separate folder:

"build": "wp-scripts build ./src/index.js ./src/timeslot/index.js",

After upgrading @wordpress/scripts from 17 to 19 running either build or start would build/bundle only the last of the files listed. After reading through #34264 I found that using --entry is a way to get it working again:

"build": "wp-scripts build --entry ./src/index.js ./src/timeslot/index.js"

So this could be seen as a documentation issue and expounding in the docs to cover this nuance may be the easiest "fix". An important note being that using --entry the built JS and CSS filenames will change (to main.*). At present, there doesn't seem to be an easy way to preserve the index filenames (noted: #34264 (comment)).

I think ideally scripts could fixed to preserve how things worked with v17. Though maybe it's more effort than what the seemingly uncommon breakage is worth and the better approach is updating the docs (and ideally the changelog to mention the specific breakage around multiple entry points that have the same filename).

Step-by-step reproduction instructions

  1. In a project with a build command like the following:
{
	"scripts": {
		"build": "wp-scripts build ./src/index.js ./src/timeslot/index.js"
	}
}
  1. Execute npm run build
  2. Note that webpack only reports on building the latter of the two entry points and the built js is not actually a bundle of all entry points.

Screenshots, screen recording, code snippet

@wordpress/scripts 17.1.0 and webpack 4

In an project I haven't bothered to upgrade I tested the build
image

wordpress/scripts 19.1.0 and webpack 5

In a separate project created for testing, it can be seen that only the latter entry is actually built:
image

Environment info

No response

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

Yes

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

No

@stokesman stokesman added the [Package] Scripts /packages/scripts label Nov 15, 2021
@gziolo gziolo added the Needs Testing Needs further testing to be confirmed. label Nov 18, 2021
@gziolo
Copy link
Member

gziolo commented Nov 22, 2021

The build creates two entry points:
Screenshot 2021-11-22 at 15 57 00

It makes me wonder whether you started using @wordpress/scripts after we upgraded webpack from v4 to v5 but before we brought back the legacy behavior for backward compatibility in v18.0.1:

https://github.com/WordPress/gutenberg/blob/trunk/packages/scripts/CHANGELOG.md#1801-2021-09-09

Related PR: #34264.

If you want to bundle all the passed scripts together then --entry should work, but the caveat is that the file gets named main.js.

@gziolo gziolo removed the Needs Testing Needs further testing to be confirmed. label Nov 22, 2021
@stokesman stokesman changed the title Scripts: Multiple entry points breakage Nov 22, 2021
@stokesman
Copy link
Contributor Author

Thanks taking time to test this @gziolo 🙇 . I apologize that I missed the crucial point to reproduce this and that is using
entry points that have the same filename and are in separate folders. I've updated the title and description to clarify and added some screenshots.

@gziolo
Copy link
Member

gziolo commented Nov 22, 2021

Thanks taking time to test this @gziolo 🙇 . I apologize that I missed the crucial point to reproduce this and that is using
entry points that have the same filename and are in separate folders. I've updated the title and description to clarify and added some screenshots.

Thank you for providing more details. I will give it a spin tomorrow and report back on what I discovered. As far as I can tell the fact that both entry points use the same name index.js is probably the reason why it behaves differently between those two versions of @wordpress/scripts. Internally, they get mapped to the same entry point named index so only one of them gets included in the build. The only question is how it worked in the past 🤔

@gziolo
Copy link
Member

gziolo commented Nov 24, 2021

Thanks taking time to test this @gziolo 🙇 . I apologize that I missed the crucial point to reproduce this and that is using
entry points that have the same filename and are in separate folders. I've updated the title and description to clarify and added some screenshots.

Thank you for providing more details. I will give it a spin tomorrow and report back on what I discovered. As far as I can tell the fact that both entry points use the same name index.js is probably the reason why it behaves differently between those two versions of @wordpress/scripts. Internally, they get mapped to the same entry point named index so only one of them gets included in the build. The only question is how it worked in the past 🤔

I can confirm that the issue is coming from the exact same name used for the file. One way to go around it looks as follows:

npx wp-scripts build ./src/index.js timeslot=./src/timeslot/index.js

It will create two entry points in the build folder:

  • index.js
  • timeslot.js

So the difference is that @wordpress/scripts@17.1.0 bundles those files together.

@stokesman
Copy link
Contributor Author

Thanks again @gziolo. The projects in which I ran into this are all parent/child blocks so I prefer the single bundle and using --entry seems the best upgrade path. That it changes the built filenames (including CSS) isn't a really an issue. If it were, there's also the option to use the single default entry point (src/index.js) and inside it import the other directories’ indexes.

I'm thinking it'd be a good idea to update the docs by changing the custom build and start examples to something like:

"build:custom": "wp-scripts build ./src/one/ ./src/two/ --output-path=custom"

As I've since learned that webpack will use the directory name as the name for the entry point. Though, if it's better to be more explicit:

"build:custom": "wp-scripts build one=./src/one/index.js two=./src/two/index.js --output-path=custom"

That may prevent new projects getting hung up on this.

@gziolo
Copy link
Member

gziolo commented Nov 29, 2021

In #25188, we discuss several options to make the support for multiple blocks more automated. Maybe the best way to move forward is to settle on a few basic patterns for detecting entry points. It looks like webpack has changed the opinion too drastically on handling multiple paths provided between webpack v4 and v5. It's tough to come up with a good recommendation now.

@stokesman, have you tested?

"build:custom": "wp-scripts build ./src/one/ ./src/two/ --output-path=custom"

Does it produce two entry points custom/one.js and custom/two.js?

@stokesman
Copy link
Contributor Author

Yes, I've tested and the output is as you wrote—custom/one.js and custom/two.js.

@gziolo
Copy link
Member

gziolo commented Dec 2, 2021

@stokesman, should we update the docs in @wordpress/scripts to include your example in addition to what is already there and provide some explanation of how it works? I'm trying to figure out what needs to happen so we could close this issue 😄

@gziolo gziolo added the [Type] Developer Documentation Documentation for developers label Dec 2, 2021
@kimmenbert
Copy link

It seems related. I'm using a custom webpack config file that looks like this:

const WordPressConfig = require( '@wordpress/scripts/config/webpack.config' );
const path = require( 'path' );
const glob = require( 'glob' );

/**
 * Help make webpack entries to the correct format with name: path
 * Modify name to exclude path and file extension
 *
 * @param {Object} paths
 */
const entryObject = ( paths ) => {
	const entries = {};

	paths.forEach( function ( filePath ) {
		let fileName = filePath.split( '/' ).slice( -1 )[ 0 ];
		fileName = fileName.replace( /\.[^/.]+$/, '' );

		if ( ! fileName.startsWith( '_' ) ) {
			entries[ fileName ] = filePath;
		}
	} );

	return entries;
};

/**
 * Extend the default WordPress/Scripts webpack to make entries and output more dynamic.
 * This checks the assts/js and assets/scss folder for any .js* and .scss files and compiles those to separate files
 *
 * Latest @Wordpress/Scripts webpack config: https://github.com/WordPress/gutenberg/blob/master/packages/scripts/config/webpack.config.js
 */
module.exports = {
	...WordPressConfig,
	entry: entryObject( glob.sync( './assets/{sass,js}/*.{scss,js*}' ) ),
	output: {
		filename: '[name].js',
		path: path.resolve( process.cwd(), 'build' ),
	},
};

You could possibly do some magic to the filename in output to add sub folder name to the output etc?

@gziolo gziolo added this to Triage in Core JS via automation May 9, 2022
@gziolo gziolo moved this from Triage to To do in Core JS May 9, 2022
@ryanwelcher ryanwelcher self-assigned this Apr 13, 2023
@stokesman
Copy link
Contributor Author

I’m closing this as water under the bridge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Scripts /packages/scripts [Type] Developer Documentation Documentation for developers
4 participants