Skip to content

Throw a clear error when a callback is passed as the first argument of configureImageRule()/configureFontRule() - #1519

Open
Amoifr wants to merge 1 commit into
symfony:mainfrom
Amoifr:fix-1221-callback-as-first-arg
Open

Throw a clear error when a callback is passed as the first argument of configureImageRule()/configureFontRule()#1519
Amoifr wants to merge 1 commit into
symfony:mainfrom
Amoifr:fix-1221-callback-as-first-arg

Conversation

@Amoifr

@Amoifr Amoifr commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
Q A
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #1221
License MIT

configureImageRule() and configureFontRule() take (options, ruleCallback). When a callback is passed as the first argument, for example:

Encore.configureFontRule(rule => {
    rule.oneOf[1].generator.filename = 'whatever/[name][ext]';
});

the function lands in the options parameter. Object.keys(aFunction) is empty, so no option validation fails, and the callback is silently dropped: the rule is never customized. This is a common mistake because several other Encore.configure*() methods take a single callback.

This PR turns that silent no-op into a helpful error pointing at the correct 2-argument form:

configureFontRule() expects an options object as its first argument. To pass only a callback, use configureFontRule({}, callback).

Only broken calls are affected (a function is never a valid options value), so there is no BC impact on working code.

This addresses the most surprising part of #1221 (the callback being ignored without any feedback). Note that overriding the filename does work today through the documented 2-argument callback by modifying the nested rule.oneOf[1].generator; setting rule.generator at the top level has no effect because Encore wraps the asset rule in a oneOf. Happy to follow up on that ergonomics point separately if you think it is worth improving.

@carsonbot carsonbot added Bug Bug Fix Status: Needs Review Needs to be reviewed labels Jul 17, 2026
@Amoifr
Amoifr force-pushed the fix-1221-callback-as-first-arg branch from 98cf44b to 1e0cb73 Compare July 17, 2026 08:42
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Jul 17, 2026
@Kocal

Kocal commented Jul 23, 2026

Copy link
Copy Markdown
Member

Hi, and thanks for opening this PR!

I just merged #1521, and with TypeScript types definitions, I'm not sure this kind of PR are still needed (same for existing type validation at runtime).

WDYT?

@Amoifr

Amoifr commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! I'd say this one is still useful: types only help people whose editor type-checks their webpack.config.js, but the recipe generates a plain JS file with no @ts-check and no compile step, so for most users nothing flags the mistake and the callback is still silently dropped at runtime. Since the failure mode is a silent no-op rather than an error, I think the runtime guard keeps its value as a safety net under the types. That said, you know the roadmap best, so if you'd rather rely on types going forward I won't fight for it. 😄

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

Labels

Bug Bug Fix Status: Reviewed Has been reviewed by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overriding the file name for "configureFontRule" or "configureImageRule" doesn't work when using a generator

4 participants