extract core and create svelte + vanilla package #8
Conversation
WalkthroughThis PR removes Tailwind imports from the CSS entry, deletes multiple public re-exports, and removes a large portion of the Yjs-backed sync system: presence, all Synced* proxy implementations, schema factory/types, utilities, and most related tests. A new core/package.json is added. Changes
Sequence Diagram(s)(omitted — changes remove features and do not introduce a new control-flow to diagram) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
core/package.json (1)
23-23: Consider safer publish workflow.The
patchscript doesn't fail on build errors (due to shell&&continuation) and skips running tests before publishing. Consider adding test execution and ensuring failures halt the process.Apply this diff:
- "patch": "npm version patch && npm run build && npm publish" + "patch": "npm run test && npm version patch && npm run build && npm publish"
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.coderabbit.ymlis excluded by none and included by none
📒 Files selected for processing (1)
core/package.json(1 hunks)
🔇 Additional comments (1)
core/package.json (1)
36-36: No runtime imports of alien-signals found
Import appears only in tests (core/tests/fixture.ts), soalien-signalsis correctly placed in devDependencies.
| @@ -0,0 +1,41 @@ | |||
| { | |||
| "name": "@syncrostate/core", | |||
| "module": "index.ts", | |||
There was a problem hiding this comment.
Fix module field to point to built output.
The module field points to the source file index.ts, while exports, main, and types correctly reference the built output in dist/. For a published package, all entry points should reference built artifacts.
Apply this diff:
- "module": "index.ts",
+ "module": "./dist/index.js",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "module": "index.ts", | |
| "module": "./dist/index.js", |
🤖 Prompt for AI Agents
In core/package.json around line 3, the "module" field currently points to the
source file "index.ts"; change it to point to the built output (e.g.,
"dist/index.js") to match "exports", "main", and "types" which reference dist/,
ensuring all package entry points reference the compiled artifacts before
publishing.
| "typescript": "^5" | ||
| }, | ||
| "dependencies": { | ||
| "@vitest/coverage-v8": "^3.2.4", |
There was a problem hiding this comment.
Move test coverage tool to devDependencies.
@vitest/coverage-v8 is a testing utility and should be in devDependencies, not dependencies. Runtime consumers of @syncrostate/core don't need coverage tooling.
Apply this diff:
"dependencies": {
- "@vitest/coverage-v8": "^3.2.4",
"esm-env": "^1.2.2",
"y-protocols": "^1.0.6",
"yjs": "^13.6.27"
},
"devDependencies": {
"@vitest/ui": "^3.2.4",
+ "@vitest/coverage-v8": "^3.2.4",
"alien-signals": "^3.0.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@vitest/coverage-v8": "^3.2.4", | |
| ++ b/core/package.json | |
| @@ -26,8 +26,7 @@ | |
| "dependencies": { | |
| - "@vitest/coverage-v8": "^3.2.4", | |
| "esm-env": "^1.2.2", | |
| "y-protocols": "^1.0.6", | |
| "yjs": "^13.6.27" | |
| @@ -33,7 +32,8 @@ | |
| "devDependencies": { | |
| "@vitest/ui": "^3.2.4", | |
| "@vitest/coverage-v8": "^3.2.4", | |
| "alien-signals": "^3.0.0", | |
| "tsdown": "...", | |
| "typescript": "...", |
🤖 Prompt for AI Agents
In core/package.json around line 29, the package "@vitest/coverage-v8" is
currently listed under "dependencies" but it is a testing tool and should be
moved to "devDependencies"; update package.json by removing the
"@vitest/coverage-v8" entry from the "dependencies" section and add the same
entry with the same version string to "devDependencies", then run npm/yarn
install (or update lockfile) to ensure dependency state is consistent.
Summary by CodeRabbit
New Features
Refactor
Chores
Style
Tests