Conversation
🦋 Changeset detectedLatest commit: 8b40ecc The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
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 |
📝 WalkthroughWalkthroughThis PR introduces deterministic sorting of entity arrays in project definitions. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 Tip You can disable the changed files summary in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/project-builder-lib/src/tools/sort-entity-arrays.unit.test.ts (1)
126-132: Use independent model instances for each container in the determinism test.
modelsCBAcurrently reuses the same objects frommodelsABC, which can hide coupling if helper code mutates model inputs. Prefer constructing a fresh reversed set forcontainer2.Suggested tweak
- const modelsABC = [ + const modelsABC = [ createTestModel({ name: 'Alpha', featureRef: testFeature.name }), createTestModel({ name: 'Bravo', featureRef: testFeature.name }), createTestModel({ name: 'Charlie', featureRef: testFeature.name }), ]; - const modelsCBA = [modelsABC[2], modelsABC[1], modelsABC[0]]; + const modelsCBA = [ + createTestModel({ name: 'Charlie', featureRef: testFeature.name }), + createTestModel({ name: 'Bravo', featureRef: testFeature.name }), + createTestModel({ name: 'Alpha', featureRef: testFeature.name }), + ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/project-builder-lib/src/tools/sort-entity-arrays.unit.test.ts` around lines 126 - 132, The determinism test reuses the same model objects by assigning modelsCBA = [modelsABC[2], modelsABC[1], modelsABC[0]] which masks mutations; instead construct fresh model instances for the reversed set by calling createTestModel for each element (using the same name and featureRef values) so modelsCBA contains independent objects; update the test setup that defines modelsABC and modelsCBA (and any reference to container2) to build modelsCBA via createTestModel invocations in reversed order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/format-and-lint.ts`:
- Around line 76-86: getPrettierConfig currently caches by config file path only
(prettierConfigCache) which causes incorrect results when
resolveConfig(filePath) applies different overrides per file type; change the
caching strategy so the cache key includes the file-specific discriminator
(e.g., file extension) or remove the cache and always call resolveConfig.
Specifically, in getPrettierConfig and where prettierConfigCache is used,
compute a key like `${configFile}::${extension}` (using path.extname(filePath))
before checking/setting the cache, or alternatively cache only the raw config
file contents from resolveConfigFile and call resolveConfig(filePath, { config:
configFile }) every time to apply overrides per-file.
---
Nitpick comments:
In `@packages/project-builder-lib/src/tools/sort-entity-arrays.unit.test.ts`:
- Around line 126-132: The determinism test reuses the same model objects by
assigning modelsCBA = [modelsABC[2], modelsABC[1], modelsABC[0]] which masks
mutations; instead construct fresh model instances for the reversed set by
calling createTestModel for each element (using the same name and featureRef
values) so modelsCBA contains independent objects; update the test setup that
defines modelsABC and modelsCBA (and any reference to container2) to build
modelsCBA via createTestModel invocations in reversed order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4e4b2119-5652-4987-986f-43511d94e856
⛔ Files ignored due to path filters (4)
examples/todo-with-better-auth/apps/admin/baseplate/generated/.paths-metadata.jsonis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/backend/baseplate/generated/.paths-metadata.jsonis excluded by!**/generated/**,!**/generated/**examples/todo-with-better-auth/apps/web/baseplate/generated/.paths-metadata.jsonis excluded by!**/generated/**,!**/generated/**tests/simple/project-definition.jsonis excluded by!tests/**
📒 Files selected for processing (22)
.changeset/sort-entity-arrays.mdexamples/blog-with-auth/baseplate/project-definition.jsonexamples/todo-with-better-auth/apps/admin/.paths-metadata.jsonexamples/todo-with-better-auth/apps/backend/.paths-metadata.jsonexamples/todo-with-better-auth/apps/web/.paths-metadata.jsonexamples/todo-with-better-auth/baseplate/project-definition.jsonpackages/project-builder-lib/src/definition/project-definition-container.tspackages/project-builder-lib/src/references/definition-ref-builder.tspackages/project-builder-lib/src/references/definition-ref-registry.tspackages/project-builder-lib/src/references/extend-parser-context-with-refs.tspackages/project-builder-lib/src/schema/features/feature.tspackages/project-builder-lib/src/schema/libraries/library.tspackages/project-builder-lib/src/schema/models/enums.tspackages/project-builder-lib/src/schema/models/models.tspackages/project-builder-lib/src/schema/plugins/definition.tspackages/project-builder-lib/src/schema/project-definition.tspackages/project-builder-lib/src/tools/index.tspackages/project-builder-lib/src/tools/sort-entity-arrays.tspackages/project-builder-lib/src/tools/sort-entity-arrays.unit.test.tsplugins/plugin-payments/src/stripe/core/schema/plugin-definition.tsplugins/plugin-payments/src/stripe/core/schema/schema-issue-checker.tsscripts/format-and-lint.ts
| const prettierConfigCache = new Map<string, Options | null>(); | ||
|
|
||
| async function getPrettierConfig(filePath: string): Promise<Options | null> { | ||
| const configFile = await resolveConfigFile(filePath); | ||
| if (!configFile) return null; | ||
| if (prettierConfigCache.has(configFile)) { | ||
| return prettierConfigCache.get(configFile) ?? null; | ||
| } | ||
| const config = await resolveConfig(filePath, { config: configFile }); | ||
| prettierConfigCache.set(configFile, config); | ||
| return config; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Prettier resolveConfig overrides behavior
💡 Result:
Prettier's resolveConfig(filePath) API resolves the effective configuration options for the given filePath by searching up the directory tree from that file's location for a configuration file (e.g., .prettierrc). It merges base options from the config file with any matching overrides based on the file's path and extension, returning a single flat Options object containing the final resolved settings (or null if no config found). The raw overrides array is not returned; instead, matching override options override base options. Key behaviors: - Requires a specific file path (not directory) to correctly apply overrides, as overrides depend on filename/extension (e.g., "*.test.js"). - Passing absolute paths may have had bugs in older versions (e.g., issue #3005 from 2017), but current versions handle them properly when the path is valid. - Resolution order: Prettier defaults -> EditorConfig (if enabled) -> config file base options -> matching overrides. - Usage example: const options = await prettier.resolveConfig('/path/to/file.js'); then prettier.format(code, { ...options, filepath: '/path/to/file.js' }); This matches how Prettier CLI and editor integrations resolve configs per file.
Citations:
- 1: https://prettier.io/docs/en/next/configuration.html
- 2: https://prettier.io/docs/configuration
- 3: https://tessl.io/registry/tessl/npm-prettier/files/docs/configuration.md
- 4: https://prettier.io/docs/api
- 5: https://prettier.io/docs/api/
- 6:
resolveConfig(cwd)fails to find config file prettier/prettier#15879 - 7: Config overrides do not work for files in the parent directory prettier/prettier#13556
- 8: resolveConfigFile bug? prettier/prettier#16344
🏁 Script executed:
cat -n scripts/format-and-lint.ts | head -150Repository: halfdomelabs/baseplate
Length of output: 6057
🏁 Script executed:
cat -n scripts/format-and-lint.ts | sed -n '150,225p'Repository: halfdomelabs/baseplate
Length of output: 2960
Prettier config cache doesn't account for file-specific overrides.
When processing multiple files from the same project, the cache keys by config file path only. However, resolveConfig(filePath) returns the merged configuration including any overrides that match the specific file's extension. With different file types (e.g., foo.ts and bar.tsx) that share a config file but match different override patterns, the second file receives the cached config resolved for the first file instead of its own.
Consider caching by filePath extension, or removing the cache entirely (config resolution is fast), or cache only the raw config file content and apply overrides per-file.
🔧 Proposed fix: cache by config file + file extension
-const prettierConfigCache = new Map<string, Options | null>();
+const prettierConfigCache = new Map<string, Options | null>();
async function getPrettierConfig(filePath: string): Promise<Options | null> {
const configFile = await resolveConfigFile(filePath);
if (!configFile) return null;
- if (prettierConfigCache.has(configFile)) {
- return prettierConfigCache.get(configFile) ?? null;
+ const ext = path.extname(filePath);
+ const cacheKey = `${configFile}::${ext}`;
+ if (prettierConfigCache.has(cacheKey)) {
+ return prettierConfigCache.get(cacheKey) ?? null;
}
const config = await resolveConfig(filePath, { config: configFile });
- prettierConfigCache.set(configFile, config);
+ prettierConfigCache.set(cacheKey, config);
return config;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/format-and-lint.ts` around lines 76 - 86, getPrettierConfig currently
caches by config file path only (prettierConfigCache) which causes incorrect
results when resolveConfig(filePath) applies different overrides per file type;
change the caching strategy so the cache key includes the file-specific
discriminator (e.g., file extension) or remove the cache and always call
resolveConfig. Specifically, in getPrettierConfig and where prettierConfigCache
is used, compute a key like `${configFile}::${extension}` (using
path.extname(filePath)) before checking/setting the cache, or alternatively
cache only the raw config file contents from resolveConfigFile and call
resolveConfig(filePath, { config: configFile }) every time to apply overrides
per-file.
kingston
deleted the
kingston/eng-1132-add-structured-ordering-to-project-definition
branch
1 participant
Summary by CodeRabbit
New Features
Chores