Conversation
This package will contain more internal util functions.
* Introduced new label configuration for the `shared` package in the GitHub Actions labeler. * This allows for better automation and organization of pull requests related to the shared package.
* Moved the `@ucdjs/utils` path below `@ucdjs/shared` for better organization. * Ensured all utility packages are grouped together for clarity.
* Changed dependency reference in `package.json` and `pnpm-lock.yaml` to reflect the new shared package. * Ensures consistency across the project by using the updated package structure.
* The `safeJsonParse` function has been removed from the utils package and re-exported from the shared package. * This change centralizes the JSON parsing utility, promoting code reuse across packages.
- Deleted the flatten.ts utility function and its associated tests. - Updated index.ts to remove the export of flattenFilePaths and added a new internal function for logging. - Removed filter.test.ts and flatten.test.ts as they are no longer needed. - Adjusted index.test.ts to test the new internal logging function. - Updated tsdown.config.ts to remove unnecessary entry. - Updated pnpm-lock.yaml to reflect changes in dependencies, replacing @ucdjs/fetch with @ucdjs/shared.
…@ucdjs/shared` * Updated imports in multiple files to use the shared package for `flattenFilePaths`. * This change promotes consistency and centralizes utility functions in the shared package.
🦋 Changeset detectedLatest commit: b280007 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
WalkthroughIntroduces a new internal package Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App/Tests
participant Store as @ucdjs/ucd-store
participant Shared as @ucdjs/shared
note over Shared: New internal package
App->>Store: import { createPathFilter, flattenFilePaths, safeJsonParse }
Store->>Shared: uses createPathFilter / flattenFilePaths / safeJsonParse
Shared-->>Store: returns utilities
Store-->>App: functions operate as before (sources migrated)
par Utils package
participant Utils as @ucdjs/utils
App->>Utils: import { createPathFilter, PRECONFIGURED_FILTERS }
Utils->>Shared: re-export from @ucdjs/shared
App->>Utils: call internal_bingbong()
Utils-->>App: logs "Bing Bong"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
pkg: ucd-store
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new @ucdjs/shared package to consolidate internal utilities and restructures the existing @ucdjs/utils package to focus on user-facing functionality. The change implements better separation between internal and external utilities within the UCD monorepo.
- Creates a new
@ucdjs/sharedpackage containing internal utilities likesafeJsonParse, path filtering, and file flattening functions - Migrates internal dependencies from
@ucdjs/utilsto@ucdjs/sharedacross multiple packages - Removes most utility functions from
@ucdjs/utils, keeping only a minimal public interface
Reviewed Changes
Copilot reviewed 29 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/* |
Complete new package setup with utilities moved from @ucdjs/utils |
packages/utils/* |
Simplified to only export from @ucdjs/shared and added test placeholder |
packages/ucd-store/* |
Updated imports to use @ucdjs/shared instead of @ucdjs/utils |
packages/fs-bridge/* |
Updated test imports to use @ucdjs/shared |
tooling/tsconfig/base.json |
Added path mapping for new @ucdjs/shared package |
.changeset/* |
Changelog entries documenting the package restructuring |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Preview Deployment for WebThe Web worker has been deployed successfully. Preview URL: https://preview.ucdjs.dev This preview was built from commit b280007 🤖 This comment will be updated automatically when you push new commits to this PR. |
Preview Deployment for ApiThe Api worker has been deployed successfully. Preview URL: https://preview.api.ucdjs.dev This preview was built from commit b280007 🤖 This comment will be updated automatically when you push new commits to this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/filter.ts (1)
1-160: Minor cleanup after migration to @ucdjs/sharedAll code imports have been updated to pull
createPathFilter,flattenFilePaths, etc., from@ucdjs/shared, and onlypackages/sharedretains a runtime dependency onpicomatch. A couple of housekeeping tasks remain:
packages/utils/README.md (around lines 22 and 51)
• Change bothimport { createPathFilter } from "@ucdjs/utils";to
import { createPathFilter } from "@ucdjs/shared";packages/fetch/package.json
• Remove the unused devDependency on"@types/picomatch": "…"since only
packages/sharedneeds Picomatch types now.
🧹 Nitpick comments (22)
packages/shared/LICENSE (1)
1-22: MIT license text looks good; minor nit on year format.Content matches the standard MIT template. Consider using a single year or a year range with a hyphen (e.g., "2025–present") to align with common conventions, though what you have is legally fine.
packages/shared/src/filter.ts (5)
18-25: Freeze PRECONFIGURED_FILTERS at runtime.You already use
as constfor type-level immutability. Freezing at runtime prevents accidental mutation in consumers.Apply:
-export const PRECONFIGURED_FILTERS = { +export const PRECONFIGURED_FILTERS = Object.freeze({ /** Excludes files containing "Test" in their name (e.g., DataTest.txt, TestFile.js) */ EXCLUDE_TEST_FILES: "!**/*Test*", /** Excludes ReadMe.txt files from any directory */ EXCLUDE_README_FILES: "!**/ReadMe.txt", /** Excludes all HTML files */ EXCLUDE_HTML_FILES: "!**/*.html", -} as const; +} as const);
27-33: Separate internal predicate type from public function type for clarity.
PathFilterFnexposes an optionalextraFiltersarg, butinternal__createFilterFunctionreturns a 1‑arg predicate. This is type-safe today, yet making the distinction explicit improves readability and prevents future refactors from passingextraFiltersinto the internal function by mistake.Apply:
type PathFilterFn = (path: string, extraFilters?: string[]) => boolean; +// Internal, pure matcher that never handles `extraFilters` +type PathPredicate = (path: string) => boolean; + export interface PathFilter extends PathFilterFn { extend: (additionalFilters: string[]) => void; patterns: () => string[]; } @@ -function internal__createFilterFunction(filters: string[], options: FilterOptions): PathFilterFn { +function internal__createFilterFunction(filters: string[], options: FilterOptions): PathPredicate { if (filters.length === 0) { return () => true; }Also applies to: 111-115
50-56: Clarify “ALL files are included” note.Given default exclusions for .zip/.pdf, consider rephrasing to “All files are included by default, subject to default exclusions” to avoid confusion.
68-76: Minor doc nit: remove unnecessary escapes in glob examples.In TS strings you don’t need to escape forward slashes. Using
!**/*.test.*,!**/node_modules/**,!**/dist/**reads cleaner.Example:
-const srcFilter = createPathFilter(['src/**', '!**\/*.test.*']); +const srcFilter = createPathFilter(['src/**', '!**/*.test.*']); @@ -const excludeFilter = createPathFilter(['!**\/node_modules/**', '!**\/dist/**']); +const excludeFilter = createPathFilter(['!**/node_modules/**', '!**/dist/**']);Also applies to: 74-78
90-97: Optional: avoid recomputing matchers for per-call extraFilters via a tiny cache.Each call with
extraFiltersrebuilds a new filter, which can be hot in loops. If this path is exercised frequently, cache by a stable key ofextraFiltersfor the life of this filter instance.Sketch:
- Keep a
Map<string, PathFilter>keyed byextraFilters.join('\u0000').- Return cached function when available.
I can send a concrete diff if you want this optimization.
packages/shared/turbo.json (1)
1-15: Turbo config is fine; consider wiring build graph.Optionally add
dependsOn: ["^build"]so this package rebuilds when upstream libs change, and declare typecheck inputs/outputs to maximize caching.Example:
"tasks": { "build": { + "dependsOn": ["^build"], "outputs": ["dist/**"] }, "dev": { "persistent": false }, "typecheck": { + "inputs": ["tsconfig.json", "tsconfig.build.json", "src/**/*.ts"], "outputs": [".cache/tsbuildinfo.json"] } }packages/shared/src/flatten.ts (1)
35-37: Guard against missing children on directory nodes.
If a directory node lackschildren,flattenFilePaths(file.children, fullPath)will throw due to the array check at Line 26. Defaulting to[]improves robustness without changing behavior for well-formed inputs.Apply this diff:
- if (file.type === "directory") { - paths.push(...flattenFilePaths(file.children, fullPath)); + if (file.type === "directory") { + paths.push(...flattenFilePaths(file.children ?? [], fullPath));.changeset/beige-dogs-roll.md (1)
5-9: Add a concise migration map for removed utilitiesInclude a short list mapping moved exports to their new import paths (e.g., safeJsonParse, flattenFilePaths → @ucdjs/shared) to ease upgrades and reduce churn for downstreams.
I can generate a migration snippet by scanning exports in utils/shared and drafting the before/after import table. Want me to add that?
packages/shared/src/json.ts (1)
1-16: Avoid console side-effects in a low-level parse utilityLibrary code should not emit console noise by default; it can leak details and makes consumers’ logs noisy. Return null silently and let callers decide how to log/handle.
Apply this diff:
export function safeJsonParse<T>(content: string): T | null { try { return JSON.parse(content) as T; - } catch (err) { - console.error("[safeJsonParse] Failed to parse JSON:", err); - return null; - } + } catch { + return null; + } }Optional follow-up for stronger type-safety (no breaking change required): add a validator-based variant so callers can enforce runtime shape.
// New helper (can live alongside safeJsonParse) export function safeJsonParseWith<T>( content: string, refine: (v: unknown) => v is T ): T | null { try { const v = JSON.parse(content) as unknown; return refine(v) ? v : null; } catch { return null; } }packages/shared/package.json (4)
21-27: Harden exports to include types via conditional exports (modern ESM).Current exports expose only JS; consumers relying on "exports" won't get types unless they also read the top-level "types" field. Adding a "types" condition improves DX and aligns with TS resolver behavior.
Apply:
- "exports": { - ".": "./dist/index.js", - "./package.json": "./package.json" - }, + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js", + "default": "./dist/index.js" + }, + "./package.json": "./package.json" + },
28-30: Whitelist README and LICENSE in published files.With "files" set, npm will usually still include README and LICENSE, but being explicit avoids surprises and makes publint happier.
"files": [ - "dist" + "dist", + "README.md", + "LICENSE" ],
34-40: Ensure builds exist on publish.Add a prepack to build and run publint before publishing. This prevents publishing an empty package when "dist" isn't checked in.
"scripts": { "build": "tsdown --tsconfig=./tsconfig.build.json", "dev": "tsdown --watch", "clean": "git clean -xdf dist node_modules", "lint": "eslint .", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "prepack": "pnpm run build && publint" },
28-33: Enable tree-shaking hints.Mark package as side-effect free for bundlers.
"files": [ "dist", "README.md", "LICENSE" ], + "sideEffects": false, "engines": { "node": ">=22.17" },packages/shared/README.md (3)
7-7: Correct package description to avoid implying fs-bridge implementations live here.Shared contains utilities; fs bridges live in @ucdjs/fs-bridge.
-A collection of utility functions and filesystem bridge implementations for the UCD project. +A collection of shared utilities for the UCD project.
15-59: Consider documenting other exported utilities (flattenFilePaths, safeJsonParse).Quick stubs improve discoverability and help downstream maintainers migrate from @ucdjs/utils.
If you want, I can open a follow-up PR to add docs with examples and edge cases.
61-65: Clarify preconfigured filter semantics with examples.Minor wording tweaks reduce ambiguity, especially around case and file variants.
-Available filters: -- `EXCLUDE_TEST_FILES`: Excludes files containing "Test" in their name -- `EXCLUDE_README_FILES`: Excludes ReadMe.txt files -- `EXCLUDE_HTML_FILES`: Excludes all HTML files +Available filters (examples): +- `EXCLUDE_TEST_FILES`: Excludes common test files (e.g., `**/*.test.*`, files under `__tests__/`). +- `EXCLUDE_README_FILES`: Excludes README files (e.g., `ReadMe.txt`, `README.md`). +- `EXCLUDE_HTML_FILES`: Excludes `**/*.html` files.Please confirm these examples match the actual patterns in packages/shared/src/filter.ts; adjust if they differ.
packages/utils/test/index.test.ts (1)
4-12: Simplify the test and harden cleanup.No need for dynamic import here; also assert the call count and always restore the spy.
Apply this diff:
-describe("bing bong internal", () => { - it("should log 'Bing Bong'", async () => { - const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - - const { internal_bingbong } = await import("../src/index").then((m) => m); - - internal_bingbong(); - - expect(consoleSpy).toHaveBeenCalledWith("Bing Bong"); - consoleSpy.mockRestore(); - }); -}); +describe("bing bong internal", () => { + it("should log 'Bing Bong'", () => { + const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); + try { + // Prefer static import at top of file if you don’t switch to a private module path. + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { internal_bingbong } = require("../src/_internal"); + internal_bingbong(); + expect(consoleSpy).toHaveBeenCalledTimes(1); + expect(consoleSpy).toHaveBeenCalledWith("Bing Bong"); + } finally { + consoleSpy.mockRestore(); + } + }); +});packages/shared/test/json.test.ts (3)
6-6: Prefer single-quoted JS strings for JSON literals to avoid escaping noise.This improves readability and reduces escape clutter.
- const validJson = "{\"name\":\"test\",\"value\":123}"; + const validJson = '{"name":"test","value":123}'; @@ - const validJson = "[1,2,3,\"test\"]"; + const validJson = '[1,2,3,"test"]'; @@ - const invalidJson = "{name:\"test\",value:123}"; // missing quotes around property names + const invalidJson = '{name:"test",value:123}'; // missing quotes around property names @@ - const incompleteJson = "{\"name\":\"test\","; + const incompleteJson = '{"name":"test",'; @@ - const nestedJson = "{\"user\":{\"name\":\"test\",\"id\":123},\"active\":true}"; + const nestedJson = '{"user":{"name":"test","id":123},"active":true}';Also applies to: 12-12, 18-18, 24-24, 30-30
1-4: Silence noisy console.error in tests and assert it in failure cases.safeJsonParse logs on parse failures; spy to keep test output clean and optionally assert the error path.
-import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; @@ describe("safeJsonParse", () => { + let errorSpy: ReturnType<typeof vi.spyOn>; + beforeEach(() => { + errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + }); + afterEach(() => { + errorSpy.mockRestore(); + }); @@ it("should return null for invalid JSON", () => { const invalidJson = '{name:"test",value:123}'; // missing quotes around property names const result = safeJsonParse(invalidJson); expect(result).toBeNull(); + expect(errorSpy).toHaveBeenCalled(); }); @@ it("should return null for incomplete JSON", () => { const incompleteJson = '{"name":"test",'; const result = safeJsonParse(incompleteJson); expect(result).toBeNull(); + expect(errorSpy).toHaveBeenCalled(); });Also applies to: 17-21, 23-27
2-2: Prefer using the published package entry in shared testsIn packages/shared/test/json.test.ts (line 2), you’re importing the module under test directly from source:
import { safeJsonParse } from "../src/index";• Since your tsconfig (tooling/tsconfig/base.json) already maps
"@ucdjs/shared"topackages/shared/src/index.ts, you can instead write:import { safeJsonParse } from "@ucdjs/shared";This approach exercises the package’s export map and will catch any regressions in your published surface.
• If these tests are meant as strict unit tests of internal implementation details, keeping the relative import is fine; otherwise, switching to the alias import is recommended.packages/utils/src/index.ts (1)
4-8: Tighten lint disables and add an explicit return type for clarity.Use a scoped lint disable for console and declare the return type; drop the broad rule disable.
-// eslint-disable-next-line ts/explicit-function-return-type -export function internal_bingbong() { - // eslint-disable-next-line no-console - console.log("Bing Bong"); -} +/** + * @internal simple diagnostic hook used in tests + */ +export function internal_bingbong(): void { + // eslint-disable-next-line no-console + console.log("Bing Bong"); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
.changeset/beige-dogs-roll.md(1 hunks).changeset/khaki-plums-return.md(1 hunks).changeset/tangy-days-wait.md(1 hunks).github/labeler.yml(1 hunks)packages/fs-bridge/package.json(1 hunks)packages/fs-bridge/test/bridges/http.test.ts(1 hunks)packages/fs-bridge/test/bridges/node.test.ts(1 hunks)packages/shared/LICENSE(1 hunks)packages/shared/README.md(1 hunks)packages/shared/eslint.config.js(1 hunks)packages/shared/package.json(1 hunks)packages/shared/src/filter.ts(2 hunks)packages/shared/src/flatten.ts(1 hunks)packages/shared/src/index.ts(1 hunks)packages/shared/src/json.ts(1 hunks)packages/shared/test/json.test.ts(1 hunks)packages/shared/tsconfig.build.json(1 hunks)packages/shared/tsconfig.json(1 hunks)packages/shared/tsdown.config.ts(1 hunks)packages/shared/turbo.json(1 hunks)packages/ucd-store/package.json(1 hunks)packages/ucd-store/src/internal/files.ts(1 hunks)packages/ucd-store/src/store.ts(2 hunks)packages/ucd-store/test/file-operations.test.ts(1 hunks)packages/utils/package.json(1 hunks)packages/utils/src/index.ts(1 hunks)packages/utils/test/index.test.ts(1 hunks)packages/utils/tsdown.config.ts(0 hunks)tooling/tsconfig/base.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/utils/tsdown.config.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: luxass
PR: ucdjs/ucd#131
File: tooling/eslint-plugin/package.json:0-0
Timestamp: 2025-07-20T05:37:40.565Z
Learning: In the ucdjs/ucd project, internal tooling packages (private packages in the tooling/ directory) export TypeScript files directly without requiring a build step, unlike published packages which use tsdown to build and export from ./dist/. Examples include ucdjs/tsdown-config and ucdjs/eslint-plugin.
📚 Learning: 2025-06-29T11:20:13.668Z
Learnt from: luxass
PR: ucdjs/ucd#87
File: packages/worker-shared/tsconfig.build.json:1-4
Timestamp: 2025-06-29T11:20:13.668Z
Learning: In the ucdjs/ucd project, the packages use tsdown instead of tsc for building libraries. The tsconfig.build.json files are primarily for IDE experience and type checking, not for direct compilation, so including "test" directories in these configs doesn't affect build output.
Applied to files:
packages/shared/tsconfig.build.jsonpackages/fs-bridge/package.jsonpackages/utils/package.jsonpackages/shared/tsconfig.jsonpackages/shared/tsdown.config.tstooling/tsconfig/base.json
📚 Learning: 2025-07-20T05:37:40.565Z
Learnt from: luxass
PR: ucdjs/ucd#131
File: tooling/eslint-plugin/package.json:0-0
Timestamp: 2025-07-20T05:37:40.565Z
Learning: In the ucdjs/ucd project, internal tooling packages (private packages in the tooling/ directory) export TypeScript files directly without requiring a build step, unlike published packages which use tsdown to build and export from ./dist/. Examples include ucdjs/tsdown-config and ucdjs/eslint-plugin.
Applied to files:
packages/shared/tsconfig.build.jsonpackages/ucd-store/package.jsonpackages/fs-bridge/package.jsonpackages/utils/package.jsonpackages/shared/tsconfig.jsonpackages/shared/tsdown.config.tstooling/tsconfig/base.jsonpackages/ucd-store/src/store.tspackages/shared/package.json
📚 Learning: 2025-06-09T05:10:32.105Z
Learnt from: luxass
PR: ucdjs/ucd#45
File: packages/ucd-store/src/download.ts:24-24
Timestamp: 2025-06-09T05:10:32.105Z
Learning: In the ucd-store package refactor, picomatch was moved from direct usage in download.ts to internal usage within the createPathFilter function in filter.ts. The pattern format is still picomatch-compatible, so JSDoc comments referencing picomatch pattern format remain correct.
Applied to files:
packages/ucd-store/package.jsonpackages/shared/src/flatten.tspackages/ucd-store/test/file-operations.test.tspackages/shared/src/filter.tspackages/shared/README.mdpackages/ucd-store/src/store.tspackages/ucd-store/src/internal/files.tspackages/utils/src/index.ts
📚 Learning: 2025-06-14T05:20:20.149Z
Learnt from: luxass
PR: ucdjs/ucd#56
File: packages/utils/src/ucd-files/validate.ts:115-130
Timestamp: 2025-06-14T05:20:20.149Z
Learning: Node.js 22 supports the `recursive` option in `fs.readdir()` (introduced in Node.js 18.17.0), which returns a flattened string array of all descendant files and directories when `recursive: true` is passed.
Applied to files:
packages/shared/src/flatten.ts
📚 Learning: 2025-07-13T09:23:43.820Z
Learnt from: luxass
PR: ucdjs/ucd#107
File: apps/api/src/routes/v1_files.ts:18-32
Timestamp: 2025-07-13T09:23:43.820Z
Learning: UNICODE_STABLE_VERSION from luxass/unicode-utils-new refers to a specific Unicode version string (not a dynamic value), and the validation logic in the files API correctly checks the normalized version (after "latest" substitution) against UNICODE_VERSION_METADATA rather than the mappedVersion from resolveUCDVersion().
Applied to files:
packages/ucd-store/test/file-operations.test.ts
🧬 Code graph analysis (4)
packages/shared/test/json.test.ts (2)
packages/shared/src/index.ts (1)
safeJsonParse(4-4)packages/shared/src/json.ts (1)
safeJsonParse(9-16)
packages/shared/tsdown.config.ts (1)
tooling/tsdown-config/src/factory.ts (1)
createTsdownConfig(25-31)
packages/shared/src/json.ts (1)
packages/shared/src/index.ts (1)
safeJsonParse(4-4)
packages/utils/test/index.test.ts (1)
packages/utils/src/index.ts (1)
internal_bingbong(5-8)
🪛 LanguageTool
packages/shared/README.md
[grammar] ~61-~61: There might be a mistake here.
Context: ...E_HTML_FILES ]); ``` Available filters: - EXCLUDE_TEST_FILES: Excludes files containing "Test" in th...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...es files containing "Test" in their name - EXCLUDE_README_FILES: Excludes ReadMe.txt files - `EXCLUDE_H...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...README_FILES: Excludes ReadMe.txt files - EXCLUDE_HTML_FILES`: Excludes all HTML files ## 📄 License...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...: Excludes all HTML files ## 📄 License Published under MIT License....
(QB_NEW_EN)
🪛 Biome (2.1.2)
tooling/tsconfig/base.json
[error] 15-15: Expected a property but instead found '// utility packages'.
Expected a property here.
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 16-16: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 17-17: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (20)
packages/shared/tsconfig.json (1)
1-5: TS config aligns with repo conventions.Extends the shared base, includes src and tests, and excludes dist. Matches prior learnings about tsdown handling builds. No issues.
packages/shared/tsconfig.build.json (1)
1-5: Build-time tsconfig is minimal and correct.Extends base.build, scopes to src, excludes dist. This fits the tsdown workflow where tsc is for types/IDE. LGTM.
packages/shared/src/filter.ts (1)
137-148: Confirm case-insensitive matching is intended across platforms.
nocase: truemakes matches case-insensitive even on case-sensitive filesystems. If patterns like*.TXTshould not matchfile.txton POSIX, consider making this configurable (defaulting to current behavior for backward compatibility).Would you like a PR option (e.g.,
FilterOptions.picomatch?: picomatch.Options) to override matcher flags? I can wire that in without breaking changes.packages/shared/src/flatten.ts (1)
13-13: Docs: example import updated to @ucdjs/shared — looks good.
Matches the new package split and will help consumers migrate smoothly.packages/ucd-store/package.json (1)
51-51: Verification complete: no lingering@ucdjs/utilsimports
Ranrg -n --type=ts -C2 -g 'packages/ucd-store/**' -P '\@ucdjs/utils'against theucd-storedirectory; no matches were found. The switch to@ucdjs/sharedis safe to merge.packages/fs-bridge/package.json (1)
52-52: @ucdjs/shared devDependency usage is correctThe verification shows that:
- No runtime code under packages/fs-bridge/src/ imports @ucdjs/shared.
- All usages are confined to test files:
- packages/fs-bridge/test/bridges/node.test.ts
- packages/fs-bridge/test/bridges/http.test.ts
Since @ucdjs/shared is only used in tests, keeping it as a devDependency is appropriate.
.github/labeler.yml (1)
35-37: Label “pkg: shared” added — good coverage for the new package.
Keeps triage consistent with other package labels.packages/shared/eslint.config.js (1)
1-7: LGTM on flat ESLint config wiringConfig matches the repo’s flat-config style and correctly sets { type: "lib", pnpm: true }. No issues found.
packages/shared/tsdown.config.ts (2)
1-7: Config is correct and explicit entry is finecreateTsdownConfig with an explicit entry of ./src/index.ts aligns with tsdown defaults and the shared package surface.
1-7: Exports configuration verified – no changes required– packages/shared/package.json correctly points its
main,module,types, andexportsentries at./dist/….
– tsdown.config.ts’sentry: ["./src/index.ts"]is correct and will produce the expecteddist/index.jsanddist/index.d.ts.No further adjustments needed here.
.changeset/tangy-days-wait.md (1)
1-12: LGTM: clear introduction of the internal shared packageMessaging is crisp about internal status and scope. No changes requested.
packages/shared/package.json (1)
31-33: Consistent Node engine requirement across packages
The Node.js engine floor is set to “>=22.17” in every package that specifies anengines.nodefield—from the rootpackage.jsonthroughpackages/sharedand all other published packages. No outliers were identified. You can leave the engines spec inpackages/shared/package.jsonas-is.packages/fs-bridge/test/bridges/http.test.ts (1)
3-3: Import source migration verified – no stale imports detected.
A repository-wide search across.ts,.tsx,.js,.mjs, and.cjsfiles for any imports offlattenFilePaths,safeJsonParse, orcreatePathFilterfrom@ucdjs/utilsreturned no results. The new import from@ucdjs/sharedis the sole reference. Changes are approved.packages/ucd-store/test/file-operations.test.ts (1)
5-5: Approve migration of file-operations tests to @ucdjs/shared
- Verified that running the provided ripgrep command returns no remaining imports of
flattenFilePaths,safeJsonParse, orcreatePathFilterfrom@ucdjs/utilsanywhere underpackages/ucd-store/.- All tests now consistently import these utilities from
@ucdjs/shared, matching the intended migration with no behavioral changes.packages/fs-bridge/test/bridges/node.test.ts (1)
1-1: LGTM: updated import path to @ucdjs/shared.No other changes; tests still read cleanly.
packages/ucd-store/src/internal/files.ts (1)
3-3: Dependency & Export Wiring for @ucdjs/shared Verified
- In
packages/ucd-store/package.json,@ucdjs/sharedis declared as a runtime dependency ("workspace:*").- In
packages/shared/src/index.ts,flattenFilePathsis correctly re-exported.- In
tooling/tsconfig/base.json, the path alias"@ucdjs/shared": ["./packages/shared/src/index.ts"]is present.All wiring checks out—approving the changes.
packages/ucd-store/src/store.ts (1)
4-4: No residual @ucdjs/utils imports; @ucdjs/shared exports validatedVerification steps completed successfully:
- Searched throughout packages/ucd-store for any imports from “@ucdjs/utils”—no matches were found.
- Confirmed in packages/shared/src/index.ts that
• createPathFilter is exported (line 1)
• flattenFilePaths is exported (line 3)
• safeJsonParse is exported (line 4)All three utilities are properly provided by
@ucdjs/shared, and there are no remaining stragglers from@ucdjs/utils. You can proceed with the consolidation as planned.packages/utils/package.json (1)
42-43: Release hygiene confirmed for @ucdjs/utilsVerified that
packages/utils/src/index.tsnow simply re-exports the exact same public symbols—createPathFilter,PRECONFIGURED_FILTERS,FilterOptions, andPathFilter—from@ucdjs/shared, and that the existingexport function internal_bingbong()remains unchanged. The API surface has neither added nor removed exports.A patch-level changeset for
@ucdjs/utilsis present (see.changeset/curly-states-pick.md), so release hygiene is satisfied and no further version bump is required.packages/shared/src/index.ts (1)
1-4: Public API surface looks good.Type-only exports are correctly separated; value exports are minimal and focused. No issues spotted.
packages/utils/src/index.ts (1)
1-2: Check backwards-compatibility of removed utils exportsWe searched the monorepo and confirmed:
- No internal code imports
flattenFilePathsorsafeJsonParsefrom@ucdjs/utils.- These utilities are consumed directly from
@ucdjs/sharedelsewhere (e.g. ucd-store, fs-bridge).- The
utilspackage is currently at version 0.1.0 (pre-1.0).Removing the direct exports of
flattenFilePathsandsafeJsonParsefrom@ucdjs/utilswill not break internal packages, but will break any external consumers who imported them from@ucdjs/utils. You have two paths forward:• If you intend to maintain a stable public API until a 1.0.0 release, add these pass-through exports now:
export { createPathFilter, PRECONFIGURED_FILTERS } from "@ucdjs/shared"; export type { FilterOptions, PathFilter } from "@ucdjs/shared"; +export { flattenFilePaths, safeJsonParse } from "@ucdjs/shared";• Otherwise, treat this as a breaking change: document it in your CHANGELOG, bump
utilsto 1.0.0, and remove the exports.Please verify whether any downstream projects rely on these exports and decide on the appropriate semver strategy.
1 participant
🔗 Linked issue
#210
📚 Description
Summary by CodeRabbit