Skip to content

[wrangler] Don't error when find_additional_modules discovers a file matching a shadowed rule#14388

Merged
petebacondarwin merged 4 commits into
mainfrom
fix/find-additional-modules-fallthrough
Jun 22, 2026
Merged

[wrangler] Don't error when find_additional_modules discovers a file matching a shadowed rule#14388
petebacondarwin merged 4 commits into
mainfrom
fix/find-additional-modules-fallthrough

Conversation

@petebacondarwin

@petebacondarwin petebacondarwin commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #14257.

Module rules assign module types to imported files — they are not include/exclude filters. Previously, when find_additional_modules walked the filesystem and discovered a file whose only matching rule had been shadowed by an earlier rule of the same type (via fallthrough: false, or by being a default rule overridden by a user-defined rule), wrangler would throw and fail the build.

For example, with this config and no_bundle: true, any .txt / .html / .sql file under the module root that did not match the user's glob would break the build, even when that file was never imported:

// wrangler.json
{
	"rules": [
		{
			"type": "Text",
			"globs": ["html/includeme.html"],
			"fallthrough": false
		}
	]
}

With this change, discovered files that only match a shadowed rule are silently skipped, and a debug-level log records each skip with a hint on how to include the file if desired (e.g. add fallthrough: true to the earlier rule or broaden the user rule's globs). The direct-import path in module-collection.ts is unchanged: importing a file in code that only matches a shadowed rule is still a hard error, since the imported file genuinely needs a defined module type.

Verified locally against the reporter's reproduction:

$ wrangler deploy --dry-run --outdir=dist
Attaching additional modules:
┌─────────────────────┬──────┬──────────┐
│ Name                │ Type │ Size     │
├─────────────────────┼──────┼──────────┤
│ html/includeme.html │ text │ 0.02 KiB │
├─────────────────────┼──────┼──────────┤
│ Total (1 module)    │      │ 0.02 KiB │
└─────────────────────┴──────┴──────────┘

  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: the prior behavior was a bug — the public docs do not describe fallthrough: false as a way to filter out files, and the only user-visible difference is that a misleading error is no longer raised for unimported files.

Open in Devin Review

@changeset-bot

changeset-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0f1669e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
wrangler Patch
@cloudflare/vite-plugin Patch
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation Bot moved this to Untriaged in workers-sdk Jun 22, 2026
@workers-devprod workers-devprod requested review from a team and edmundhung and removed request for a team June 22, 2026 16:14
@workers-devprod

workers-devprod commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

@ask-bonk

ask-bonk Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

The tests look good and cover both scenarios. The logic change is sound and well-tested. Let me do a final sanity check on whether there's any scenario where this behavioral change could be a backward-compatibility/silent-data-loss concern, and verify the second test rules get parsed with proper default rules included.

The second test passes only one user rule with fallthrough: false. The default rules get appended via parseRules. The default Text rule (**/*.html etc.) gets shadowed because it's the same type after the non-fallthrough user rule. So dontincludeme.html matches only the shadowed default rule → skipped. includeme.html matches the live user rule → included. Correct.

The change is well-scoped, correctly guarded by moduleNames.has(filePath), the direct-import error path is preserved, UserError import is still used, and tests cover both cases. The only nit (multiple debug logs per file when matching multiple shadowed globs) is cosmetic and at debug level.

This is a clean, correct fix.

LGTM

github run

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

All changesets look good

@pkg-pr-new

pkg-pr-new Bot commented Jun 22, 2026

Copy link
Copy Markdown
@cloudflare/autoconfig

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/autoconfig@14388

create-cloudflare

npm i https://pkg.pr.new/cloudflare/workers-sdk/create-cloudflare@14388

@cloudflare/deploy-helpers

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/deploy-helpers@14388

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/kv-asset-handler@14388

miniflare

npm i https://pkg.pr.new/cloudflare/workers-sdk/miniflare@14388

@cloudflare/pages-shared

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/pages-shared@14388

@cloudflare/unenv-preset

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/unenv-preset@14388

@cloudflare/vite-plugin

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/vite-plugin@14388

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/vitest-pool-workers@14388

@cloudflare/workers-auth

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/workers-auth@14388

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/workers-editor-shared@14388

@cloudflare/workers-utils

npm i https://pkg.pr.new/cloudflare/workers-sdk/@cloudflare/workers-utils@14388

wrangler

npm i https://pkg.pr.new/cloudflare/workers-sdk/wrangler@14388

commit: 0f1669e

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

devin-ai-integration[bot]

This comment was marked as resolved.

Comment thread .changeset/find-additional-modules-shadowed-rule.md Outdated
…matching a shadowed rule

Module rules assign module types to imported files — they are not include/exclude
filters. Previously, when find_additional_modules walked the filesystem and
discovered a file whose only matching rule had been shadowed by an earlier rule
of the same type (via fallthrough: false, or by being a default rule overridden
by a user rule), wrangler would throw and fail the build.

Discovered files that only match a shadowed rule are now silently skipped, with
a debug-level log explaining what happened and how to include the file if
desired. The direct-import path is unchanged: importing a file in code that
only matches a shadowed rule is still a hard error.

Fixes #14257

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 0 new potential issues.

Open in Devin Review

@workers-devprod workers-devprod left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codeowners reviews satisfied

@github-project-automation github-project-automation Bot moved this from Untriaged to Approved in workers-sdk Jun 22, 2026
@petebacondarwin petebacondarwin force-pushed the fix/find-additional-modules-fallthrough branch from bfecfa4 to 0f1669e Compare June 22, 2026 21:01

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 0 new potential issues.

Open in Devin Review

@petebacondarwin petebacondarwin merged commit 3f02864 into main Jun 22, 2026
59 checks passed
@petebacondarwin petebacondarwin deleted the fix/find-additional-modules-fallthrough branch June 22, 2026 21:12
@github-project-automation github-project-automation Bot moved this from Approved to Done in workers-sdk Jun 22, 2026
petebacondarwin added a commit that referenced this pull request Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[wrangler] Can't override default module rules using fallthrough = false

3 participants