-
-
Notifications
You must be signed in to change notification settings - Fork 62
Fix HMR not triggering for additional translation paths #215
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
base: main
Are you sure you want to change the base?
Fix HMR not triggering for additional translation paths #215
Conversation
WalkthroughThe pull request modifies the hot update handler in the Vite plugin to watch all configured language paths (both primary Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| }, | ||
| handleHotUpdate(ctx) { | ||
| if (/lang\/.*\.php$/.test(ctx.file)) { | ||
| if ([langPath, ...additionalLangPaths].some(path => new RegExp(`${path}.*\\.php$`).test(ctx.file))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escape special regex characters in path strings.
The path strings are inserted directly into the RegExp constructor without escaping. If any path contains special regex metacharacters (e.g., ., \, [, ], (, ), etc.), they will be interpreted as regex syntax rather than literal characters, causing incorrect matching behavior.
For example:
- Windows paths like
C:\Users\Project\lang\contain backslashes - Paths with dots like
lang.backupwould matchlangXbackupdue to the unescaped.
Apply this diff to escape regex special characters:
- if ([langPath, ...additionalLangPaths].some(path => new RegExp(`${path}.*\\.php$`).test(ctx.file))) {
+ if ([langPath, ...additionalLangPaths].some(path => new RegExp(`${path.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}.*\\.php$`).test(ctx.file))) {Alternatively, for better readability, you could extract a helper function:
const escapeRegex = (str: string) => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
// Then use:
if ([langPath, ...additionalLangPaths].some(path => new RegExp(`${escapeRegex(path)}.*\\.php$`).test(ctx.file))) {🤖 Prompt for AI Agents
In src/vite.ts around line 57, the RegExp is built from raw path strings so
special regex characters in langPath or additionalLangPaths (e.g., backslashes,
dots, brackets) are treated as regex metacharacters and break matching; fix by
escaping regex metacharacters in each path before interpolating into the RegExp
(implement or call a small escapeRegex helper that replaces characters like . *
+ ? ^ $ { } ( ) | [ ] \ with escaped versions) and then use the escaped path
when constructing the pattern so the test matches literal paths ending with
.php.
🧩 Analysis chain
Verify path matching behavior with absolute paths and path separators.
The implementation needs verification for several path-handling concerns:
-
ctx.file format: Vite's
ctx.fileis typically an absolute path. Ensure the pattern matching works correctly with absolute paths fromctx.file. -
Path separator consistency:
langPathhas a trailingpath.sepappended at line 10, but elements inadditionalLangPathsmay not. This inconsistency could cause:- Overly broad matches (e.g.,
langpattern would matchlang-backup/file.php) - Missing matches if separators don't align
- Overly broad matches (e.g.,
-
Test coverage: Verify the fix works with:
- Files in
langPathdirectory - Files in
additionalLangPathsdirectories - Nested subdirectories within those paths
- Both Windows and Unix path separators
- Files in
Run the following script to check how ctx.file is used in similar Vite plugins:
Consider normalizing all paths consistently:
const normalizedAdditionalPaths = additionalLangPaths.map(p => p.replace(/[\\/]$/, '') + path.sep)
if ([langPath, ...normalizedAdditionalPaths].some(path => ...)) {🏁 Script executed:
#!/bin/bash
# Search for handleHotUpdate usage patterns in similar contexts
rg -n -C3 'handleHotUpdate.*ctx\.file' --type=tsLength of output: 0
🏁 Script executed:
#!/bin/bash
# Examine the context around line 57 in src/vite.ts
cat src/vite.ts | head -80 | tail -40Length of output: 1107
🏁 Script executed:
#!/bin/bash
# Search for langPath definition and context
rg -n -B5 -A5 'langPath' src/vite.tsLength of output: 1923
🏁 Script executed:
#!/bin/bash
# Search for additionalLangPaths
rg -n -B3 -A3 'additionalLangPaths' src/vite.tsLength of output: 954
🏁 Script executed:
#!/bin/bash
# Look for the original regex pattern to compare
rg -n 'lang.*php' src/vite.tsLength of output: 110
🏁 Script executed:
#!/bin/bash
# Search for git history or any comments about the original pattern
git log -p --all -S '/lang/.*\.php$' -- src/vite.ts 2>/dev/null | head -50Length of output: 59
🏁 Script executed:
#!/bin/bash
# Check if there are any test files that show expected behavior
fd -e 'test|spec' -t f | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for handleHotUpdate in node_modules or docs to understand ctx.file
rg -n 'ctx.file' --type=ts -B2 -A2 2>/dev/null | head -30Length of output: 329
🌐 Web query:
Vite handleHotUpdate ctx.file absolute or relative path format
💡 Result:
ctx.file is a filesystem path pointing to the changed file — it is an absolute path (you usually call path.relative(root, ctx.file) or normalizePath on it when you need a project-relative or POSIX-normalized path). [1][2]
Sources:
- Vite HMR hook HmrContext (ctx.file property). [1]
- Vite plugin code/examples using ctx.file as an absolute path and calling path.relative/normalizePath. [2]
Fix path normalization and absolute path matching in handleHotUpdate.
Critical issues found during verification:
-
Absolute path mismatch:
ctx.fileis an absolute path (e.g.,/home/user/project/resources/lang/en.php), but the regex pattern uses relative paths likelang/.*\.php$. This pattern will never match because the absolute path contains the full directory structure. -
Path separator inconsistency:
langPathis normalized withpath.sepat line 10, butadditionalLangPathsare not. This causes:langPathpattern:lang/.*\.php$(correct boundary)additionalLangPathspattern:extra.*\.php$(matchesextra-dir/file.phpunintentionally)
-
Missing normalization:
additionalLangPathsmust be normalized the same way aslangPathbefore use in the regex at line 57.
Required fixes:
- Normalize all paths to absolute form for proper matching against
ctx.file - Ensure all paths in the array have consistent separators
- Use
path.resolve()to convert relative paths to absolute, or usepath.relative(root, ctx.file)to convertctx.fileto project-relative for matching
Example correction:
const pathsToCheck = [langPath, ...additionalLangPaths].map(p =>
p.replace(/[\\/]$/, '') // normalize separators
)
if (pathsToCheck.some(p => new RegExp(`${p.replace(/\\/g, '/')}.*\\.php$`).test(ctx.file.replace(/\\/g, '/')))) {Or use path.relative() for more robust matching against the absolute ctx.file.
🤖 Prompt for AI Agents
In src/vite.ts around line 57, the hot-update path test fails because ctx.file
is an absolute path while langPath and additionalLangPaths are relative and
inconsistently normalized; normalize and compare on the same basis by converting
all language paths to absolute with path.resolve(root, p) (or convert ctx.file
to project-relative with path.relative(root, ctx.file)), normalize separators
(e.g., replace backslashes with forward slashes or use path.posix) and trim any
trailing slashes, then build the regex/tests from those normalized values so the
.some(...) check compares like-with-like (absolute-to-absolute or
relative-to-relative) before testing for .*\.php$ matches.
This PR fixes the hot-reload behavior when using additional translation directories.
The current HMR validation only checks for files matching lang/.*.php$, which prevents from triggering when translations in custom paths changes.
This update replaces the hardcoded pattern with a dynamic check based on langPath and additionalLangPaths.
Please, let me know if you'd like any adjustments.
closes #212
Summary by CodeRabbit
Release Notes