feat: Bump path-to-regexp to v8 #9942
Conversation
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
|
🚀 Thanks for opening this pull request! |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughStandardizes route path handling to path-to-regexp v8: dependency bumped, cloud route strings use brace-based catch-alls, rate-limit compilation/validation now uses path-to-regexp v8, tests and docs updated to named wildcard syntax. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AddRateLimit as addRateLimit
participant Config as Config.validateRateLimit
participant P2R as pathToRegexp
participant RateHandler as handleRateLimit
Client->>AddRateLimit: register route (requestPath)
AddRateLimit->>Config: validateRateLimit(requestPath)
alt invalid v8 syntax
Config-->>AddRateLimit: throw validation error
else valid v8 pattern
Config->>P2R: pathToRegexp(requestPath)
P2R-->>Config: compiled regexp
Config-->>AddRateLimit: attach compiled regexp to limit.path.regexp
end
AddRateLimit->>RateHandler: store limit (with .path and/or .path.regexp)
Client->>RateHandler: incoming request
RateHandler->>RateHandler: match using limit.path.regexp if present else fallback to limit.path
RateHandler->>RateHandler: apply rate limiting logic
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #9942 +/- ##
==========================================
- Coverage 93.08% 93.07% -0.01%
==========================================
Files 188 188
Lines 15298 15299 +1
Branches 177 177
==========================================
Hits 14240 14240
- Misses 1046 1047 +1
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
package.json (1)
1-177: Suggested Angular-style PR title for changelogFor the changelog, an Angular-style title that reflects both the dependency bump and behavioral tweaks could be:
fix(rest): update path-to-regexp to v8.3.0 and adjust route matchingThis reads better in release notes than a plain “bump” and hints at the routing impact.
src/middlewares.js (1)
564-593: Wildcard transform matches v8 syntax; consider special-casing bare'*'The new
transformPathlogic correctly:
- Leaves named wildcards like
:id*and*idintact.- Converts standalone
*in a segment to*path, which is the expected “named wildcard” form for path-to-regexp v8 (e.g./*path). (classic.yarnpkg.com)One small edge case is when
route.requestPath === '*': this currently becomes*path(no leading slash), which is probably OK but diverges from the documented examples (/*pathand/files{/*path}). If you want to be explicit and future-proof, you could special-case it:- let transformPath = route.requestPath; - // Replace * that are not part of named parameter syntax - // Use a function to check if * is part of a named parameter - transformPath = transformPath.replace(/\*/g, (match, offset, string) => { + let transformPath = route.requestPath; + + // Special-case bare "*" to clearly mean "match all paths" + if (transformPath === '*') { + transformPath = '/*path'; + } + + // Replace * that are not part of named parameter syntax + // Use a function to check if * is part of a named parameter + transformPath = transformPath.replace(/\*/g, (match, offset, string) => {If you’d like stronger assurance, you could add a small unit test matrix for
requestPathvalues like/classes/*,/classes/*id,/classes/:id*,*, and ensure the resultingpathToRegexp(transformPath).regexpmatches expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)src/cloud-code/Parse.Cloud.js(1 hunks)src/middlewares.js(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
🧬 Code graph analysis (1)
src/cloud-code/Parse.Cloud.js (1)
src/GraphQL/transformers/mutation.js (1)
parseClass(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: Node 18
- GitHub Check: Redis Cache
- GitHub Check: Node 22
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Node 20
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Code Analysis (javascript)
- GitHub Check: Docker Build
- GitHub Check: Benchmarks
🔇 Additional comments (3)
package.json (1)
53-53: path-to-regexp bump to 8.3.0 looks appropriateThe version bump aligns with current non‑vulnerable releases and the routing changes in this PR; main follow-up is just to ensure the existing routing / rate-limit test suites are run to catch any subtle behavior changes.
You may want to run the full test suite against a representative set of routes and custom rate-limit configs, especially any using wildcards or custom
requestPathvalues, to confirm nothing regresses with the new matcher behavior.src/cloud-code/Parse.Cloud.js (1)
76-91: Brace-based routes align with path-to-regexp v8; verify built-in class coverageUsing
{/*id}to make the trailing wildcard segment optional matches the v8 guidance (e.g./files{/*path}) and should work fine when fed throughpathToRegexpand the new wildcard transform. (app.unpkg.com)The only thing to double-check in tests is that the
routemapping (_User→users,_Session→sessions, etc.) plus these patterns still correctly cover all REST endpoints you intend to rate-limit (e.g. list vs. object vs. nested paths) for both built-in and custom classes.You might want a small integration test that, for a sample of classes (
_User,_Session, a custom class, and@File), asserts that the generatedrequestPathpatterns actually match the concrete URLs produced by your routers.src/middlewares.js (1)
324-337: Rewritten review comment is unnecessary—the current code is safeAll rate limits are added exclusively through
addRateLimit(line 521), which always populatespathviapathToRegexp(transformPath)(line 591). Since this is the only entry point, the fallbacklimit.path.regexp || limit.pathat line 324 correctly handles the expectedpathToRegexpobject structure. There is no code path by which a plain string could reachhandleRateLimit, making the suggested defensive guard redundant.Likely an incorrect or invalid review comment.
This sounds like a workaround; what changes are needed in the codebase to avoid doing this complex path transformation? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4556 🏁 Script executed: Length of output: 584 🏁 Script executed: Length of output: 4900 🏁 Script executed: Length of output: 596 🌐 Web query: 💡 Result: Short migration checklist for path-to-regexp v8 (what changed, and how to fix your routes)
Quick migration steps
References
If you want, I can scan your codebase (or a sample of your routes) and produce exact replacement edits. 🏁 Script executed: Length of output: 6393 🏁 Script executed: Length of output: 90 🏁 Script executed: Length of output: 26222 @mtrezza The complex transformation exists primarily for backward compatibility with user-provided rate limit configurations. Here's what's happening and the alternatives: Current SituationUsers configure rate limits with paths using the old wildcard syntax: rateLimit: {
requestPath: '/functions/*', // Old syntax
requestTimeWindow: 10000,
requestCount: 1
}The transformation in Alternatives to Avoid the Transformation1. Breaking Change - Require Users to Migrate (Cleanest approach)Force users to update their configurations to use v8 syntax: // Old (no longer supported)
requestPath: '/functions/*'
requestPath: '*'
// New (required)
requestPath: '/functions/*path'
requestPath: '*path'Changes needed:
2. Transform at Configuration Time (Simpler & earlier validation)Move the transformation to // In Config.js validateRateLimit function
static validateRateLimit(options) {
// ... existing validation ...
// Transform and validate path syntax here
option.requestPath = transformPathToV8(option.requestPath);
// Validate the transformed path works with pathToRegexp
try {
pathToRegexp(option.requestPath);
} catch (e) {
throw `Invalid requestPath: ${e.message}`;
}
}This way the transformation happens once during server initialization, and 3. Store Compiled Regex (Best performance)Instead of storing the path string and re-compiling repeatedly, compile once: // In addRateLimit (middlewares.js)
config.rateLimits.push({
path: {
original: route.requestPath,
regexp: pathToRegexp(transformPath) // Compile once, store the regex
},
handler: rateLimit({...})
});
// In handleRateLimit
const pathExp = limit.path.regexp; // Already compiled
if (pathExp.test(req.url)) { ... }This is already partially implemented (note line 324: 4. Deprecation Path (Most user-friendly)Support both syntaxes temporarily with warnings: function transformAndValidatePath(path) {
const hasOldSyntax = /(?<![:\w])\*(?![:\w])/.test(path);
if (hasOldSyntax) {
logger.warn(
`Deprecation warning: Rate limit path "${path}" uses old wildcard syntax. ` +
`Please update to path-to-regexp v8 syntax (e.g., "/*path" instead of "/*")`
);
return transformPath(path); // Apply transformation
}
return path; // Already v8 compatible
}RecommendationI'd suggest Option 2 (transform at config time) combined with Option 4 (deprecation warnings) as the best path forward:
Would you like me to search for the exact location in 🧠 Learnings used |
|
@coderabbitai, we'll release the new major version of Parse Server 9 shortly, so it would be a good opportunity to add breaking changes to the release. The option "1. Breaking Change - Require Users to Migrate (Cleanest approach)" sounds like a good candidate and the breaking change sounds simple enough, so that developers can migrate to the new rate limit syntax. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 5457 🏁 Script executed: Length of output: 4282 @mtrezza Perfect! Let's implement the breaking change for Parse Server 9. Here's a comprehensive implementation plan: Implementation Plan1. Remove Transformation Logic in
|
|
@coratgerl instead of applying a patch we should fix the underlying issue, because otherwise we're just accumulating debt that eventually comes due in the future. |
You're right, it does look better now. I assume you’ll be writing a migration guide? Should I not include these changes in my PR, then? We can add them later to the migration guide, right? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Config.js (1)
691-710: Excellent validation with clear migration guidance.The validation logic correctly identifies deprecated wildcard syntax patterns and provides helpful migration examples. The regex patterns accurately distinguish between unnamed wildcards (e.g.,
/*) and valid named wildcards (e.g.,/*path).Optional: Consider adding a comment explaining the regex patterns for future maintainers:
// Validate path-to-regexp v8 syntax // Check for common old syntax patterns const oldWildcardPattern = /(?:^|\/)\*(?:\/|$)/; // Matches unnamed wildcards like /* or */ const nakedWildcard = /^[\s]*\*[\s]*$/; // Matches bare * (with optional whitespace)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
spec/RateLimit.spec.js(16 hunks)src/Config.js(2 hunks)src/Options/index.js(1 hunks)src/middlewares.js(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/RateLimit.spec.js
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Applied to files:
src/Options/index.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: Node 20
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: Redis Cache
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Node 18
- GitHub Check: Node 22
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Docker Build
- GitHub Check: Code Analysis (javascript)
- GitHub Check: Benchmarks
🔇 Additional comments (5)
src/middlewares.js (1)
565-565: LGTM! Correct migration to path-to-regexp v8.The direct use of
pathToRegexp(route.requestPath)correctly replaces the previous wildcard transformation logic and aligns with path-to-regexp v8 requirements.src/Options/index.js (1)
353-353: LGTM! Clear documentation of the v8 syntax requirement.The updated documentation clearly explains that wildcards must be named and provides helpful examples. The reference to the upstream path-to-regexp documentation is useful.
spec/RateLimit.spec.js (3)
8-8: LGTM! Tests correctly updated for path-to-regexp v8 syntax.All test paths have been consistently updated to use named wildcards as required by path-to-regexp v8:
/functions/*→/functions/*path/classes/*→/classes/*path*→*pathAlso applies to: 29-29, 48-48, 86-86, 105-105, 125-125, 144-144, 243-243, 297-297, 323-323, 343-343, 389-389, 410-410, 505-505
376-376: Good improvement: Error object instead of string.Throwing an
Errorobject instead of a string is better practice and provides a proper stack trace.
484-491: Excellent test coverage for deprecated syntax validation.The test correctly validates that the deprecation error message is thrown when using old wildcard syntax, ensuring users receive clear migration guidance.
|
You can add a migration guide for PS9, following the style of the existing PS8 guide. |
|
@coderabbitai what is this path-to-regexp v8 syntax? Is that a custom syntax or does it follow any wider known syntax? What is it similar to, regex, glob, etc? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🌐 Web query: 💡 Result: Summary of path-to-regexp v8 pattern syntax and matching (concise)
Examples
If you want, I can fetch the exact README section or link specific examples from the v8 package files. Citations:
@mtrezza path-to-regexp uses its own custom routing pattern syntax, though it's widely adopted in the JavaScript ecosystem. Here's how it relates to other pattern systems: What is it?Custom routing syntax - path-to-regexp defines its own DSL (Domain-Specific Language) specifically for URL route matching. It's not a standard like regex or glob, but it has become a de facto standard in Node.js web frameworks. Most Similar ToExpress.js routing patterns - In fact, Express.js uses path-to-regexp internally for route matching. If you've written Express routes, you've used this syntax: // Express routing (powered by path-to-regexp)
app.get('/users/:id', handler); // Named parameter
app.get('/files/*', handler); // Wildcard (old syntax)
app.get('/posts/:id?', handler); // Optional parameterRelationship to Other Systems
Syntax ElementsThe core tokens include text, named parameters ( // Named parameters
'/users/:id' // Matches: /users/123
// Wildcards (v8 requires naming)
'/files/*path' // Matches: /files/a/b/c
// Modifiers
'/posts/:id?' // Optional: ?
'/tags/:name+' // One-or-more: +
'/categories/:slug*' // Zero-or-more: *
// Custom regex within parameters
'/user/:id(\\d+)' // Only matches digits
// Optional groups
'/users{/:id}/profile' // Matches: /users/profile OR /users/123/profileWhy It Existspath-to-regexp bridges the gap between simple string matching and full regex complexity. It provides:
Ecosystem AdoptionUsed by popular frameworks:
So while it's a custom syntax, it's become a widely-recognized standard in the JavaScript/Node.js routing ecosystem, much like how regex is a standard for general pattern matching. 🧠 Learnings used |
|
@coderabbitai will express framework also adopt the new v8 syntax, or did it already adopt it? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🌐 Web query: 💡 Result: Short summary — what happened and what to do
References
@mtrezza Yes, Express v5 has already been released and it uses path-to-regexp v8. This was an upgrade from the old 0.x/2.x versions used in Express v4. Express v5 Breaking ChangesExpress v5 developers face the same migration requirements:
Alignment with Parse Server 9This means Parse Server 9's migration to path-to-regexp v8 syntax aligns with the broader Node.js ecosystem. Developers already familiar with Express v5 migration will recognize the same patterns. The breaking change you're introducing is consistent with what Express (the most widely-used Node.js framework) has already required from their users. This makes the migration more justifiable and the new syntax more recognizable to the community. 🧠 Learnings used |
|
Parse Server migrated to express 5 some months ago, in the migration guide we can simply refer to the express migration guide which is the same topic. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/RateLimit.spec.js (1)
1-527: Consider updating the PR title to better reflect the breaking change.Based on learnings, Parse Server PRs should follow Angular commit convention with a meaningful changelog entry. Since this introduces a breaking change requiring developers to update their rate limit path syntax, consider updating the PR title to:
feat(rate-limit)!: require path-to-regexp v8 syntax for rate limit pathsThe
!indicates a breaking change, the scope clarifies the affected subsystem, and the description is action-oriented to help developers understand the impact.Based on learnings, PR titles should follow Angular convention and clearly convey the change's impact to developers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
9.0.0.md(1 hunks)spec/RateLimit.spec.js(16 hunks)src/Config.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Config.js
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Applied to files:
9.0.0.md
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Applied to files:
9.0.0.md
📚 Learning: 2025-11-17T15:02:48.786Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Applied to files:
9.0.0.md
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
spec/RateLimit.spec.js
🪛 LanguageTool
9.0.0.md
[uncategorized] ~9-~9: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...limiting) --- ## Route Path Syntax and Rate Limiting Parse Server 9 standardizes the route p...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Node 20
- GitHub Check: Node 22
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: Redis Cache
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Docker Build
- GitHub Check: Code Analysis (javascript)
- GitHub Check: Benchmarks
🔇 Additional comments (3)
spec/RateLimit.spec.js (3)
8-8: LGTM! Path syntax consistently updated to path-to-regexp v8.All rate limit path configurations have been correctly updated to use named wildcards (
*path) instead of unnamed wildcards (*), which is required by path-to-regexp v8. The changes are consistent across all test scenarios.Also applies to: 29-29, 48-48, 86-86, 105-105, 125-125, 144-144, 243-243, 297-297, 323-323, 343-343, 389-389, 410-410, 505-505
376-376: LGTM! Improved error throwing.Correctly updated to throw an Error object instead of a string, which provides better stack traces and follows best practices.
484-491: LGTM! Excellent deprecation validation with helpful migration guidance.The deprecation test correctly validates that the old wildcard syntax is rejected with a clear, actionable error message that includes concrete migration examples. This provides excellent developer experience during the upgrade.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
9.0.0.md (1)
9-9: Optional: Consider hyphenating "Rate Limiting" in the heading.The static analysis tool suggests using "Rate-Limiting" (with a hyphen) when used as a compound adjective modifying a noun. While "Rate Limiting" without the hyphen is widely accepted in technical documentation, adding the hyphen would follow strict grammar rules.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
9.0.0.md(1 hunks)src/Options/Definitions.js(1 hunks)src/Options/docs.js(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Applied to files:
src/Options/docs.jssrc/Options/Definitions.js9.0.0.md
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Applied to files:
9.0.0.md
📚 Learning: 2025-11-17T15:02:48.786Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Applied to files:
9.0.0.md
📚 Learning: 2025-11-17T15:02:24.824Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Applied to files:
9.0.0.md
🪛 LanguageTool
9.0.0.md
[uncategorized] ~9-~9: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...limiting) --- ## Route Path Syntax and Rate Limiting Parse Server 9 standardizes the route p...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Node 20
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: Node 22
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: Redis Cache
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Docker Build
- GitHub Check: Benchmarks
🔇 Additional comments (3)
9.0.0.md (1)
1-54: Excellent migration guide with clear examples.The migration guide now includes concrete before/after examples as requested in the previous review. The syntax mapping table (lines 23-28) and JavaScript configuration examples (lines 30-46) make the required changes clear and actionable for developers. The reference to the Express 5 migration guide provides helpful additional context.
Based on learnings, this addresses the previous comment requesting examples in the migration guide.
src/Options/Definitions.js (1)
681-681: Verify the documentation generation process was followed.The updated documentation text accurately describes path-to-regexp v8 syntax requirements and provides clear examples. However, since this is a generated file (as stated in lines 2-3), please confirm that:
- The source file
src/Options/index.jswas updated with these documentation changes- The command
npm run definitionswas executed to regenerate this fileBased on learnings, for new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js.
src/Options/docs.js (1)
123-123: Clear and accurate documentation for path-to-regexp v8 syntax.The updated JSDoc documentation accurately describes the path-to-regexp v8 requirements for rate limit paths. The examples (
/functions/*path,/classes/MyClass/*path,/*path) are practical and consistent with the migration guide in 9.0.0.md. The explicit requirement that "wildcards must be named" helps developers understand the breaking change from v6.
mtrezza
added
the
state:breaking
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Options/Definitions.js (1)
678-683: Rate limitrequestPathhelp text correctly documents path-to-regexp v8 named wildcards; optional deprecation noteThe updated description aligns well with path-to-regexp v8: it clearly states that wildcards must be named and gives valid examples like
/*path,/functions/*path, etc., which matches upstream docs. This is a good clarification for users configuring rate limits.If the implementation still auto-upgrades legacy patterns like
/functions/*to/functions/*pathfor backward compatibility, you may want to add a short “legacy syntax is deprecated and will be removed in a future major version” note here or in the main rate‑limit docs, so the “must be named” wording doesn’t confuse users who still see unnamed*working during the transition. This is purely a documentation polish and can be deferred.Also, per the project’s Angular-style convention, a changelog-friendly PR title that fits this change set could be:
feat(routing)!: adopt path-to-regexp v8 syntax for route paths
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.json(1 hunks)src/Options/Definitions.js(1 hunks)src/Options/docs.js(1 hunks)src/Options/index.js(1 hunks)src/middlewares.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/Options/docs.js
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/Options/index.js
- src/middlewares.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Applied to files:
src/Options/Definitions.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: Node 22
- GitHub Check: Redis Cache
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: Node 20
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: Node 18
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Docker Build
- GitHub Check: Code Analysis (javascript)
- GitHub Check: Benchmarks
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
9.0.0.md (1)
12-15: Clarify the timing and scope of validation.The "Deprecation" bullet states that old syntax "will trigger a clear error message," but doesn't specify when validation occurs (e.g., at Parse Server startup during config validation vs. at request time). Based on the PR changes, validation happens at startup; consider adding a phrase like "during Parse Server startup" to remove ambiguity and help users understand when they'll encounter the error.
Consider updating line 15 to:
-- **Deprecation**: Old wildcard route syntax is deprecated and will trigger a clear error message. +- **Deprecation**: Old wildcard route syntax is deprecated and will trigger a clear error message during Parse Server startup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
9.0.0.md(1 hunks)spec/RateLimit.spec.js(15 hunks)src/Config.js(2 hunks)src/Options/index.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- spec/RateLimit.spec.js
- src/Options/index.js
- src/Config.js
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
Applied to files:
9.0.0.md
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Applied to files:
9.0.0.md
📚 Learning: 2025-11-17T15:02:48.786Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Applied to files:
9.0.0.md
📚 Learning: 2025-11-17T15:02:24.824Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Applied to files:
9.0.0.md
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.
Applied to files:
9.0.0.md
📚 Learning: 2025-05-09T09:59:06.289Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.
Applied to files:
9.0.0.md
📚 Learning: 2025-05-04T20:41:05.147Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.
Applied to files:
9.0.0.md
📚 Learning: 2025-08-26T14:06:31.853Z
Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.
Applied to files:
9.0.0.md
🪛 LanguageTool
9.0.0.md
[uncategorized] ~9-~9: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...limiting) --- ## Route Path Syntax and Rate Limiting Parse Server 9 standardizes the route p...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: PostgreSQL 18, PostGIS 3.6
- GitHub Check: PostgreSQL 17, PostGIS 3.5
- GitHub Check: Redis Cache
- GitHub Check: Node 22
- GitHub Check: MongoDB 6, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.3
- GitHub Check: PostgreSQL 16, PostGIS 3.5
- GitHub Check: PostgreSQL 15, PostGIS 3.4
- GitHub Check: Node 20
- GitHub Check: MongoDB 7, ReplicaSet
- GitHub Check: MongoDB 8, ReplicaSet
- GitHub Check: PostgreSQL 15, PostGIS 3.5
- GitHub Check: Node 18
- GitHub Check: Docker Build
- GitHub Check: Code Analysis (javascript)
- GitHub Check: Benchmarks
🔇 Additional comments (1)
9.0.0.md (1)
1-56: Documentation is comprehensive and well-structured.The migration guide successfully addresses the path-to-regexp v8 transition with clear examples, actionable steps, and external references. Past feedback requesting concrete before/after examples has been properly incorporated (lines 19–46), and the note linking to external documentation (lines 52–54) provides helpful context for users.
3 participants
Pull Request
Issue
Fixes: #9402
This code snippet transforms wildcard characters (
*) in route paths into named parameters compatible with path-to-regexp v8. The transformation is necessary because path-to-regexp v8 no longer supports unnamed wildcards (*) due to security and design changes.Key Logic
route.requestPathfor any*characters.*(e.g.,/files/*→/files/*path).*in named parameters (e.g.,:id*or*id) to avoid breaking existing functionality.Why?
Example
Summary by CodeRabbit
Chores
Refactor
Bug Fixes / Validation
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.