[dynamic-import-chunkname] Disallow chunk name when eager mode is enabled

[amsardesai]

The dynamic-import-chunkname rule is useful for catching missing chunk names for imports, however much of our imports use webpackMode: "eager" to disable creating a separate chunk (i.e for lazy evaluation). Adding webpackChunkName anyway can force webpack to generate a chunk, defeating the purpose of the eager mode. This PR changes the rule to disallow chunk names when webpackMode: "eager" is set.

Related issue: #1904

[ljharb]

When would you not want this option enabled?

[amsardesai]

Never, really. I'll just make it the default.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.82%. Comparing base (c0ac54b) to head (c8eb588).

❗ Current head c8eb588 differs from pull request most recent head 7173569. Consider uploading reports for the commit 7173569 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3004      +/-   ##
==========================================
+ Coverage   95.66%   95.82%   +0.15%     
==========================================
  Files          78       78              
  Lines        3275     3279       +4     
  Branches     1150     1151       +1     
==========================================
+ Hits         3133     3142       +9     
+ Misses        142      137       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ljharb]

Is there anyone who this could break? I assume not, because the only complaint would be that it fails to warn on something?

[amsardesai]

Exactly, this only suppresses existing warnings in a narrow case, and doesn't introduce new warnings.

[ljharb]

Thanks, this is great!

One more enhancement we may want to consider before landing this: when webpackMode is eager, and a useless name is present, could we give a suggestion to remove the name?

[amsardesai]

@ljharb Yeah that makes sense to me, just updated the PR to warn when chunk name and eager mode are both set.

[ljharb]

Suggested change

	if (isChunknamePresent && isEagerModePresent) {
	context.report({
	node,
	message:
	`dynamic imports should not have both webpackChunkName and webpackMode: "eager"`,
	});
	}
	if (isChunknamePresent && isEagerModePresent) {
	context.report({
	node,
	message: 'dynamic imports should not have both webpackChunkName and webpackMode: "eager"`\',
	});
	}

also is there any reason why someone might still want a chunk name? if so, then this should be a suggestion instead of a warning

[amsardesai]

I don't think having a chunk name with eager mode makes sense, since eager mode explicitly disables creating a new chunk, so the module is just lazily evaluated instead of lazily loaded+evaluated. Adding a chunk name anyways basically reverts back to being webpackMode: 'lazy' and generates a new chunk. A warning would help users catch unintentional additional chunks when using webpackMode: 'eager'

[ljharb]

ok, so having both is always a mistake. thanks!

[ljharb]

altho actually - that means that removing the name would quietly change the mode from lazy to eager - i think this should be a suggestion to either remove the name, or remove/change the webpackMode, to be safe.

[amsardesai]

makes sense, i'll make that change!

[ljharb]

@amsardesai sorry if i wasn't clear; a "suggestion" isn't prose giving the user a hint, it's an actual eslint feature where the editor will prompt them with multiple autofixes they have to explicitly choose. For an example, look for a rule with hasSuggestions: true in its schema.

[amsardesai]

Ah, I didn't realize you meant ESLint suggestions. Let me do that.

[amsardesai]

Hey @ljharb, would love to know if this can be merged now, or if any other changes need to be made!