Integrate internal_helpers for Astro and update dependencies#1580
Open
Adammatthiesen wants to merge 5 commits into
Open
Integrate internal_helpers for Astro and update dependencies#1580Adammatthiesen wants to merge 5 commits into
Adammatthiesen wants to merge 5 commits into
Conversation
…tion feat(markdoc, markdown-remark, md, mdx, wysiwyg, component-registry): migrate to use internal_helpers for virtual imports fix(studiocms): replace deprecated Astro integration kit helpers chore: update package dependencies to include internal_helpers test(internal_helpers): add tests for virtualImportsPlugin functionality
🦋 Changeset detectedLatest commit: 4b94f81 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 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 |
Contributor
📝 WalkthroughWalkthroughChangesShared Astro helper migration
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Contributor
Allure Test Report for this PR:✅ Allure Report | History |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
create-studiocms
effectify
studiocms
@studiocms/auth0
@studiocms/blog
@studiocms/cloudinary-image-service
@studiocms/devapps
@studiocms/discord
@studiocms/github
@studiocms/google
@studiocms/html
@studiocms/markdoc
@studiocms/markdown-remark
@studiocms/md
@studiocms/mdx
@studiocms/s3-storage
@studiocms/upgrade
@studiocms/wysiwyg
@withstudiocms/api-spec
@withstudiocms/auth-kit
@withstudiocms/cli-kit
@withstudiocms/component-registry
@withstudiocms/config-utils
@withstudiocms/effect
@withstudiocms/internal_helpers
@withstudiocms/kysely
@withstudiocms/sdk
@withstudiocms/template-lang
commit: |
Adammatthiesen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
.changeset/seven-socks-wink.md (1)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExpand changeset description with rationale/impact.
The description is a single line. Based on learnings, changesets should describe the change, rationale, and potential impact so maintainers understand versioning decisions during initial development. Consider noting that this removes the deprecated
astro-integration-kitdependency in favor of internal helpers, with no expected user-facing behavior change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/seven-socks-wink.md around lines 1 - 6, Expand the changeset description to state that the deprecated astro-integration-kit dependency is being replaced with internal helpers, include the rationale for the replacement, and note that no user-facing behavior change is expected.Source: Learnings
packages/@withstudiocms/internal_helpers/src/vite/virtualImportsPlugin.ts (1)
29-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo test coverage for the ambiguous-
ssrfallback branch.
matchesContext's "backward compatibility" branch (context defined,options.ssrundefined) is untested —packages/@withstudiocms/internal_helpers/test/virtualImportsPlugin.test.tsonly exercises unscoped imports and explicitssr: true/falsecases. Given the correctness/security concern raised above, this branch should have explicit test coverage to lock in intended behavior once resolved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/`@withstudiocms/internal_helpers/src/vite/virtualImportsPlugin.ts around lines 29 - 58, In virtualImportsPlugin tests, add explicit coverage for matchesContext when an import defines a context but the plugin hook receives options with ssr undefined. Assert the intended backward-compatible fallback behavior for both matching and non-matching contexts, using the existing virtualImportsPlugin test patterns and symbols.packages/@withstudiocms/internal_helpers/test/virtualImportsPlugin.test.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
@ts-nocheckmakes the subsequent type assertion inert.The file disables all type checking, so the
as Exclude<ReturnType<typeof virtualImportsPlugin>, undefined>casts (Lines 18, 48, 87) provide no actual type safety — they're purely documentation at that point. As per coding guidelines,**/*.{test.ts,test.tsx,spec.ts,spec.tsx}files should "Write unit tests with proper TypeScript types, mock external dependencies, and cover plugin integration and auth flows." Consider removing@ts-nocheckand narrowingpluginwith a proper type guard/non-null assertion instead, so type errors in the test itself are still caught.Also applies to: 16-18
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/`@withstudiocms/internal_helpers/test/virtualImportsPlugin.test.ts at line 1, Remove the file-level `@ts-nocheck` directive in virtualImportsPlugin tests so TypeScript validates the test code and its ReturnType-based assertions. Update the plugin handling around the casts at the affected test cases to narrow the possibly undefined result with an appropriate type guard or non-null assertion, while preserving the existing test behavior.Source: Coding guidelines
packages/@studiocms/md/vitest.config.ts (1)
1-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate test-integration scaffold across packages.
This entire
test-integrationblock (including thestudiocms:component-registry/runtimemock) is duplicated verbatim inpackages/@studiocms/mdx/vitest.config.ts. Consider extracting it into the sharedvitest.shared.jshelper referenced at line 5, reducing duplicate maintenance when this mock needs to change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/`@studiocms/md/vitest.config.ts around lines 1 - 20, Extract the duplicated testIntegration AstroIntegration scaffold, including the studiocms:component-registry/runtime mock, into the shared vitest helper used by configShared. Update both package Vitest configurations to reuse that shared symbol and remove their local test-integration definitions, preserving the existing test behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.changeset/seven-socks-wink.md:
- Around line 1-6: Expand the changeset description to state that the deprecated
astro-integration-kit dependency is being replaced with internal helpers,
include the rationale for the replacement, and note that no user-facing behavior
change is expected.
In `@packages/`@studiocms/md/vitest.config.ts:
- Around line 1-20: Extract the duplicated testIntegration AstroIntegration
scaffold, including the studiocms:component-registry/runtime mock, into the
shared vitest helper used by configShared. Update both package Vitest
configurations to reuse that shared symbol and remove their local
test-integration definitions, preserving the existing test behavior.
In `@packages/`@withstudiocms/internal_helpers/src/vite/virtualImportsPlugin.ts:
- Around line 29-58: In virtualImportsPlugin tests, add explicit coverage for
matchesContext when an import defines a context but the plugin hook receives
options with ssr undefined. Assert the intended backward-compatible fallback
behavior for both matching and non-matching contexts, using the existing
virtualImportsPlugin test patterns and symbols.
In `@packages/`@withstudiocms/internal_helpers/test/virtualImportsPlugin.test.ts:
- Line 1: Remove the file-level `@ts-nocheck` directive in virtualImportsPlugin
tests so TypeScript validates the test code and its ReturnType-based assertions.
Update the plugin handling around the casts at the affected test cases to narrow
the possibly undefined result with an appropriate type guard or non-null
assertion, while preserving the existing test behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6b96ab2d-e598-4b89-800e-e874cf6b1b72
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
.changeset/eager-mirrors-write.md.changeset/eight-meals-invent.md.changeset/seven-socks-wink.mdpackages/@studiocms/blog/package.jsonpackages/@studiocms/blog/src/index.tspackages/@studiocms/blog/vitest.config.tspackages/@studiocms/devapps/package.jsonpackages/@studiocms/devapps/src/index.tspackages/@studiocms/html/package.jsonpackages/@studiocms/html/vitest.config.tspackages/@studiocms/markdoc/package.jsonpackages/@studiocms/markdoc/src/index.tspackages/@studiocms/markdoc/vitest.config.tspackages/@studiocms/markdown-remark/src/integration/index.tspackages/@studiocms/markdown-remark/src/integration/integration-utils.tspackages/@studiocms/md/package.jsonpackages/@studiocms/md/src/index.tspackages/@studiocms/md/vitest.config.tspackages/@studiocms/mdx/package.jsonpackages/@studiocms/mdx/src/index.tspackages/@studiocms/mdx/vitest.config.tspackages/@studiocms/wysiwyg/package.jsonpackages/@studiocms/wysiwyg/vitest.config.tspackages/@withstudiocms/component-registry/package.jsonpackages/@withstudiocms/component-registry/src/registry/handler.tspackages/@withstudiocms/internal_helpers/package.jsonpackages/@withstudiocms/internal_helpers/src/astro-integration/addVirtualImports.tspackages/@withstudiocms/internal_helpers/src/astro-integration/index.tspackages/@withstudiocms/internal_helpers/src/vite/index.tspackages/@withstudiocms/internal_helpers/src/vite/virtualImportsPlugin.tspackages/@withstudiocms/internal_helpers/test/virtualImportsPlugin.test.tspackages/studiocms/package.jsonpackages/studiocms/src/handlers/astroConfigCheck.tspackages/studiocms/src/handlers/changelog.tspackages/studiocms/src/handlers/pluginHandler.tspackages/studiocms/src/handlers/routeHandler.tspackages/studiocms/src/handlers/setupDbStudio.tspackages/studiocms/src/index.tspackages/studiocms/src/integrations/dynamic-sitemap/index.tspackages/studiocms/src/integrations/node-namespace.tspackages/studiocms/src/plugins/analytics/index.tspackages/studiocms/test/virtuals/utils.test.tspackages/studiocms/tsconfig.jsonpackages/studiocms/vitest.config.tsplayground/astro.config.mtsplayground/package.jsonpnpm-workspace.yaml
💤 Files with no reviewable changes (4)
- packages/studiocms/test/virtuals/utils.test.ts
- pnpm-workspace.yaml
- packages/@studiocms/markdown-remark/src/integration/integration-utils.ts
- packages/studiocms/tsconfig.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new shared helper for Astro integrations and migrates all packages to use it, simplifying and unifying how virtual imports are added across the codebase. It removes the old per-package helper implementations and updates dependencies accordingly. These changes improve maintainability and consistency for all packages that provide or consume virtual imports.
Migration to shared Astro integration helpers
addVirtualImportshelper in the@withstudiocms/internal_helperspackage to standardize the way virtual imports are added in Astro integrations. (.changeset/eager-mirrors-write.md, .changeset/eager-mirrors-write.mdR1-R5)@studiocms/blog,@studiocms/devapps,@studiocms/html,@studiocms/markdoc,@studiocms/markdown-remark,@studiocms/mdx,@studiocms/md,@studiocms/wysiwyg,@studiocms/component-registry) to use the new shared helper, removing the oldvirtualImportsPluginimplementations. (.changeset/eight-meals-invent.md, [1];.changeset/seven-socks-wink.md, [2]Code and configuration updates
package.jsonfiles to add@withstudiocms/internal_helpersas a dependency where needed. [1] [2] [3] [4]index.ts,vitest.config.ts) in affected packages to useaddVirtualImportsand remove legacy code. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15]Other minor changes
package.jsonexports for@studiocms/htmland@studiocms/markdoc. [1] [2]These changes collectively make the codebase more maintainable, reduce duplication, and ensure a consistent integration pattern for virtual imports across all StudioCMS packages.
@coderabbitai ignore
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests