feat:支持e2e,以及功能文档优化 #51
Conversation
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 52 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughAdds a boolean prop defaultFieldReplace and its runtime handling to the search-box; switches docs to a local VitePress dev workflow with conditional aliases; adds Playwright E2E configuration and test updates; augments docs/examples with data-source references; and updates TS shims, scripts, and build utilities. ChangesCombined changes (single cohort)
sequenceDiagram
participant Dev
participant CLI
participant VitePress
participant SearchBox
participant Playwright
Dev->>CLI: pnpm dev (local)
CLI->>VitePress: vitepress dev (USE_LOCAL_SEARCH_BOX=true)
VitePress->>SearchBox: import aliased ../search-box
SearchBox->>VitePress: register TinySearchBox + i18n
Dev->>CLI: pnpm test:playground
CLI->>Playwright: playwright test
Playwright->>VitePress: request /examples/* pages
Playwright->>SearchBox: run defaultFieldReplace scenarios
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/docs/guide/usage.md (1)
300-309:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocumentation inconsistency: "注意事项" item 3 still implies
setGlobalAppis required.The newly added "i18n 自动继承说明" section (line 303) correctly states that
setGlobalAppis usually no longer necessary. However, the existing "注意事项" bullet on line 296 still reads:如需使用国际化,请正确配置
setGlobalAppThis creates a direct contradiction for readers who encounter the "注意事项" section first.
📝 Suggested update for line 296
-3. **国际化配置**:如需使用国际化,请正确配置 `setGlobalApp` +3. **国际化配置**:组件会自动继承宿主应用的 i18n 配置,通常无需手动调用 `setGlobalApp`;多应用/微前端等特殊场景仍可手动调用🤖 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/docs/guide/usage.md` around lines 300 - 309, Update the contradictory "注意事项" bullet that currently says "如需使用国际化,请正确配置 `setGlobalApp`" to reflect the new auto-inherit behavior: change the wording to state that i18n is automatically inherited by components (per the "i18n 自动继承说明" section) and that `setGlobalApp` is only required in special scenarios (e.g., multi-app or micro-frontend), referencing the existing "i18n 自动继承说明" section for details; locate the bullet in the "注意事项" list and replace the sentence with a concise note about auto-inheritance and optional `setGlobalApp` usage.packages/docs/search-box/max-time-length.spec.ts (1)
21-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPre-existing:
.isVisibleused as a property reference instead of a method call — assertions are silent no-ops.
page.locator(...).isVisible(without()) evaluates to a function reference, not aPromise. Awaiting it resolves immediately and never validates whether the element is actually visible. The intended error-message assertions effectively do nothing.🐛 Proposed fix
- await page.getByText('可选时间跨度为366.0天内').isVisible - await page.getByTitle('值不能为空').isVisible + await expect(page.getByText('可选时间跨度为366.0天内')).toBeVisible() + await expect(page.getByTitle('值不能为空')).toBeVisible()- await page.getByText('可选时间跨度为182.5天内').isVisible - await page.getByTitle('值不能为空').isVisible + await expect(page.getByText('可选时间跨度为182.5天内')).toBeVisible() + await expect(page.getByTitle('值不能为空')).toBeVisible()Also applies to: 45-46
🤖 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/docs/search-box/max-time-length.spec.ts` around lines 21 - 22, The test currently awaits function references instead of checking visibility: change the silent no-ops by calling the visibility methods (e.g., replace page.getByText('可选时间跨度为366.0天内').isVisible and page.getByTitle('值不能为空').isVisible with actual visibility checks); either call the method (isVisible()) and await its result or, better, use Playwright assertions (await expect(page.getByText('可选时间跨度为366.0天内')).toBeVisible() and await expect(page.getByTitle('值不能为空')).toBeVisible()). Apply the same fix to the other occurrences mentioned (lines 45-46) so the assertions actually validate visibility.
🧹 Nitpick comments (5)
packages/docs/.vitepress/config.mts (1)
17-50: 💤 Low valueLocal-mode aliasing looks correct; minor note on the
vuealias.The
useLocalSearchBoxtoggle andoptimizeDeps.include/excludesplit are sound for HMR with the local source. One thing to keep in mind:vueis aliased tovue.runtime.esm-bundler.js, which lacks the runtime template compiler. If any docs example or a transitively-imported file ships pre-compiled render functions only, this is fine; otherwise users with rawtemplate:strings in dev examples would hit the "Failed to mount: template option is not available in the runtime-only build" error. Not blocking, just worth verifying once locally.🤖 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/docs/.vitepress/config.mts` around lines 17 - 50, The current Vite alias in resolve.alias maps the key 'vue' to the runtime-only build (vue/dist/vue.runtime.esm-bundler.js), which will break examples that use raw template strings; update the alias in the resolve.alias object (the 'vue' entry) to point to a build that includes the template compiler (e.g., vue/dist/vue.esm-bundler.js) or ensure all examples and transitive packages ship precompiled render functions; check the alias logic that uses useLocalSearchBox so the corrected 'vue' alias is applied in both local and non-local modes.packages/search-box/index.ts (2)
51-53: 💤 Low valueAuto-install for Vue 2 only — confirm intent.
Browser-global auto-install assumes
window.Vueis the Vue 2 constructor. Vue 3 isn't typically attached aswindow.Vueand usescreateApp(...).use(plugin)instead, so this path is effectively Vue 2-only — which is fine, but worth a comment so users don't expect auto-install in a Vue 3 UMD scenario.🤖 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/search-box/index.ts` around lines 51 - 53, The auto-install branch using window.Vue is Vue 2–specific; update the TinySearchBox auto-install block to either check Vue.version.startsWith("2") before calling TinySearchBox.install or at minimum add a clear inline comment above the block stating this is intentionally Vue 2 only and will not auto-install for Vue 3 (which uses createApp(...).use(plugin)); reference the TinySearchBox.install call and the window.Vue access so reviewers can find and update the auto-install logic or comment accordingly.
23-46: 💤 Low valueInstall flow is reasonable; small style/ergonomics nit.
The Vue 2 / Vue 3 dual-runtime detection is correct, and guarding the mixin via
__TINY_SEARCH_BOX_I18N_MIXIN__prevents repeated registration. Two minor points:
- Quote/semicolon style is inconsistent within this file (
"./src/index"and;on lines 13/19/45/51–52, vs. single quotes / no semicolons elsewhere). Prettier or the existing project style should be applied uniformly.- Consider narrowing the runtime types instead of
any(e.g., a smallVueLikeinterface) to keep type safety on the install path.Style cleanup diff
-import TinySearchBox from "./src/index"; +import TinySearchBox from './src/index' import { t, setGlobalApp } from './src/utils/i18n' import zhCN from './src/utils/zh_CN' import enUS from './src/utils/en_US' import TinySearchBoxFirstLevelPanel from './src/components/first-level-panel.vue' import TinySearchBoxSecondLevelPanel from './src/components/second-level-panel.vue' -import { version } from "./package.json"; +import { version } from './package.json' @@ - runtime.component(TinySearchBox.name, TinySearchBox); -}; + runtime.component(TinySearchBox.name, TinySearchBox) +} @@ -if (typeof window !== "undefined" && (window as typeof window & { Vue?: unknown }).Vue) { - TinySearchBox.install((window as typeof window & { Vue: any }).Vue); +if (typeof window !== 'undefined' && (window as typeof window & { Vue?: unknown }).Vue) { + TinySearchBox.install((window as typeof window & { Vue: any }).Vue) }🤖 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/search-box/index.ts` around lines 23 - 46, The file mixes quoting/semicolon styles and uses a broad any for the Vue runtime; please normalize formatting to the repository style (consistent single/double quotes and semicolon usage) throughout the file and replace the runtime: any with a narrow interface (e.g., VueLike) used in TinySearchBox.install so you retain type safety for properties/methods used (config.globalProperties, mixin, component, __TINY_SEARCH_BOX_I18N_MIXIN__, and vm.$root/$i18n/$t) while keeping the existing mixin guard and calls to setGlobalApp and runtime.component unchanged.packages/search-box/theme/index.less (1)
114-117: 💤 Low valueSpecificity hack via duplicated selector is fragile.
Repeating
.@{popper-tiny-cls}.@{popper-tiny-cls}is a workaround to outweigh upstream tinyvue styles. It works but obscures intent and breaks if the upstream selector specificity changes again. Consider:
- Co-locating this rule with the existing block at lines 18–108 (already targeting the same combined selector) so a single, intentional rule controls both
width/overflowandpadding, or- Using
:where(...)or an explicit cascade ordering so the override is documented rather than relying on duplicated class names.A short comment explaining why the duplication is necessary would also help future maintainers.
🤖 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/search-box/theme/index.less` around lines 114 - 117, The duplicated-specificity selector .@{popper-tiny-cls}.@{popper-tiny-cls}.@{search-box__dropdown-menu-cls} is a fragile hack; replace it by moving the padding:0 rule into the same rule block that already targets the combined selector for width/overflow (the existing combined selector block that uses .@{popper-tiny-cls} with .@{search-box__dropdown-menu-cls}), or refactor to an explicit specificity solution such as wrapping the selector with :where(...) or ordering the stylesheet so the override appears after the upstream rules; if you must keep the duplication, add a short comment above the rule explaining why extra specificity is required and reference the upstream source.package.json (1)
18-18: 💤 Low valueAlign
build:docswith other migrated scripts for consistency.Lines 8-17 use
pnpm -C packages/docs ...butbuild:docson line 18 still usespnpm -F@opentiny/vue-search-box-docsdocs:build. The package name inpackages/docs/package.jsonis still@opentiny/vue-search-box-docs, so both forms work, but the inconsistency makes future maintenance harder and may cause confusion.♻️ Proposed alignment
- "build:docs": "pnpm -F `@opentiny/vue-search-box-docs` docs:build", + "build:docs": "pnpm -C packages/docs docs:build",🤖 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 `@package.json` at line 18, The "build:docs" npm script is inconsistent with other migrated scripts; update the "build:docs" entry so it uses the same workspace invocation pattern (pnpm -C packages/docs docs:build) instead of the -F `@opentiny/vue-search-box-docs` form; modify the "build:docs" script in package.json (the "build:docs" key) to call pnpm -C packages/docs docs:build so it aligns with the other scripts.
🤖 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.
Inline comments:
In `@packages/docs/.vitepress/config.mts`:
- Line 63: The favicon href is currently an absolute '/favicon.ico' which
ignores the configured base and will 404 on non-root deployments; update the
head entry (the head array / the link element) so the href is base-aware—either
use a relative path like 'favicon.ico' or prefix with the Vite base via
import.meta.env.BASE_URL (e.g. build the href as import.meta.env.BASE_URL +
'favicon.ico') so VitePress rewrites the URL correctly when the site base is
non-root.
In `@packages/docs/apis/props.md`:
- Line 6: Update the table row for the property default-field-replace: change
the description to explain that when true, creating a new tag via the default
field path replaces the previous default-field tag instead of appending
(translate to Chinese accordingly), fix the Markdown table formatting by adding
the missing trailing pipe `|` at the end of the row, and adjust the column
spacing to match the surrounding rows so the table pipes align consistently with
other entries.
In `@packages/docs/README.md`:
- Around line 18-25: The README's "Optional modes" list is missing the script
entry for the defined npm script test:e2e:codegen:headed; update the list under
"Optional modes" in packages/docs/README.md to include a new line for `pnpm
test:e2e:codegen:headed` alongside the other e2e commands so the documentation
matches the package.json scripts.
In `@packages/docs/search-box/help.spec.ts`:
- Line 5: Change the test navigation to use an absolute path: locate the
page.goto call in help.spec.ts (the line with page.goto('examples/help')) and
update the argument to include a leading slash (i.e., '/examples/help') so
Playwright resolves it against the base origin like the other specs.
In `@packages/docs/search-box/max-length.spec.ts`:
- Line 8: The tests call the Playwright locator visibility property instead of
invoking it (using `.isVisible` rather than calling it) which makes assertions
no-ops; update each occurrence (e.g., the locator expression
`page.locator(...).filter(...).nth(...)` and other locators using `.isVisible`)
to use Playwright's expect assertions — replace the bare `.isVisible` usage with
`await expect(locator).toBeVisible()` (or `toBeHidden()` as appropriate) so the
visibility is actually asserted.
In `@packages/docs/search-box/potential-match.spec.ts`:
- Line 5: The test navigates to a non-existent route because
page.goto('/examples/potential-options') is incorrect; update the navigation to
the registered route page.goto('/examples/potential-match') so the spec loads
potential-match.md content and the assertions run against the intended page
(look for the page.goto call in potential-match.spec.ts / potential-match test).
In `@packages/docs/tsconfig.json`:
- Around line 20-25: The tsconfig.json include globs currently miss .mts module
files (e.g., .vitepress/config.mts), so update the "include" array to add
patterns that match .mts files—add "**/*.mts" and "**/*.d.mts" plus
".vitepress/**/*.mts" (and optionally ".vitepress/**/*.d.mts") so those files
are picked up for TypeScript checking and editor support; modify the existing
"include" entries rather than removing any existing patterns.
In `@packages/search-box/src/composables/use-dropdown.ts`:
- Around line 238-248: The current logic only sets replace when currentItem
equals the keyword fallback (state.allTypeAttri), so defaultFieldReplace is
ignored when a consumer configures defaultField; update the condition that
computes normalizedCurrentItem so defaultFieldReplace also applies when
currentItem was selected via the defaultField branch — e.g., compute a boolean
like isDefaultFallback = currentItem?.field === state.allTypeAttri.field ||
(defaultField && currentItem?.field === defaultField) and then use that to set
normalizedCurrentItem (and subsequently extract { replace, type, mergeTag,
regexp }), ensuring defaultFieldReplace is honored for both the keyword fallback
and explicit defaultField selection.
---
Outside diff comments:
In `@packages/docs/guide/usage.md`:
- Around line 300-309: Update the contradictory "注意事项" bullet that currently
says "如需使用国际化,请正确配置 `setGlobalApp`" to reflect the new auto-inherit behavior:
change the wording to state that i18n is automatically inherited by components
(per the "i18n 自动继承说明" section) and that `setGlobalApp` is only required in
special scenarios (e.g., multi-app or micro-frontend), referencing the existing
"i18n 自动继承说明" section for details; locate the bullet in the "注意事项" list and
replace the sentence with a concise note about auto-inheritance and optional
`setGlobalApp` usage.
In `@packages/docs/search-box/max-time-length.spec.ts`:
- Around line 21-22: The test currently awaits function references instead of
checking visibility: change the silent no-ops by calling the visibility methods
(e.g., replace page.getByText('可选时间跨度为366.0天内').isVisible and
page.getByTitle('值不能为空').isVisible with actual visibility checks); either call
the method (isVisible()) and await its result or, better, use Playwright
assertions (await expect(page.getByText('可选时间跨度为366.0天内')).toBeVisible() and
await expect(page.getByTitle('值不能为空')).toBeVisible()). Apply the same fix to the
other occurrences mentioned (lines 45-46) so the assertions actually validate
visibility.
---
Nitpick comments:
In `@package.json`:
- Line 18: The "build:docs" npm script is inconsistent with other migrated
scripts; update the "build:docs" entry so it uses the same workspace invocation
pattern (pnpm -C packages/docs docs:build) instead of the -F
`@opentiny/vue-search-box-docs` form; modify the "build:docs" script in
package.json (the "build:docs" key) to call pnpm -C packages/docs docs:build so
it aligns with the other scripts.
In `@packages/docs/.vitepress/config.mts`:
- Around line 17-50: The current Vite alias in resolve.alias maps the key 'vue'
to the runtime-only build (vue/dist/vue.runtime.esm-bundler.js), which will
break examples that use raw template strings; update the alias in the
resolve.alias object (the 'vue' entry) to point to a build that includes the
template compiler (e.g., vue/dist/vue.esm-bundler.js) or ensure all examples and
transitive packages ship precompiled render functions; check the alias logic
that uses useLocalSearchBox so the corrected 'vue' alias is applied in both
local and non-local modes.
In `@packages/search-box/index.ts`:
- Around line 51-53: The auto-install branch using window.Vue is Vue 2–specific;
update the TinySearchBox auto-install block to either check
Vue.version.startsWith("2") before calling TinySearchBox.install or at minimum
add a clear inline comment above the block stating this is intentionally Vue 2
only and will not auto-install for Vue 3 (which uses
createApp(...).use(plugin)); reference the TinySearchBox.install call and the
window.Vue access so reviewers can find and update the auto-install logic or
comment accordingly.
- Around line 23-46: The file mixes quoting/semicolon styles and uses a broad
any for the Vue runtime; please normalize formatting to the repository style
(consistent single/double quotes and semicolon usage) throughout the file and
replace the runtime: any with a narrow interface (e.g., VueLike) used in
TinySearchBox.install so you retain type safety for properties/methods used
(config.globalProperties, mixin, component, __TINY_SEARCH_BOX_I18N_MIXIN__, and
vm.$root/$i18n/$t) while keeping the existing mixin guard and calls to
setGlobalApp and runtime.component unchanged.
In `@packages/search-box/theme/index.less`:
- Around line 114-117: The duplicated-specificity selector
.@{popper-tiny-cls}.@{popper-tiny-cls}.@{search-box__dropdown-menu-cls} is a
fragile hack; replace it by moving the padding:0 rule into the same rule block
that already targets the combined selector for width/overflow (the existing
combined selector block that uses .@{popper-tiny-cls} with
.@{search-box__dropdown-menu-cls}), or refactor to an explicit specificity
solution such as wrapping the selector with :where(...) or ordering the
stylesheet so the override appears after the upstream rules; if you must keep
the duplication, add a short comment above the rule explaining why extra
specificity is required and reference the upstream source.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88ef79b3-4c63-448f-8b0d-93e9c59ee3d9
📒 Files selected for processing (47)
package.jsonpackages/docs/.vitepress/config.mtspackages/docs/.vitepress/theme/index.tspackages/docs/README.mdpackages/docs/apis/props.mdpackages/docs/examples/auto-match.mdpackages/docs/examples/basic-usage.mdpackages/docs/examples/default-field.mdpackages/docs/examples/empty-placeholder.mdpackages/docs/examples/help.mdpackages/docs/examples/item-placeholder.mdpackages/docs/examples/panel-max-height.mdpackages/docs/examples/potential-match.mdpackages/docs/examples/split-input-value.mdpackages/docs/examples/v-model.mdpackages/docs/guide/usage.mdpackages/docs/package.jsonpackages/docs/playwright.config.tspackages/docs/search-box/auto-match.spec.tspackages/docs/search-box/basic-usage.spec.tspackages/docs/search-box/basic-usage.vuepackages/docs/search-box/custom-panel.spec.tspackages/docs/search-box/default-field.spec.tspackages/docs/search-box/editable.spec.tspackages/docs/search-box/empty-placeholder.spec.tspackages/docs/search-box/events.spec.tspackages/docs/search-box/group-key.spec.tspackages/docs/search-box/help.spec.tspackages/docs/search-box/id-map-key.spec.tspackages/docs/search-box/max-length.spec.tspackages/docs/search-box/max-time-length.spec.tspackages/docs/search-box/merge-tag.spec.tspackages/docs/search-box/potential-match.spec.tspackages/docs/search-box/settings.spec.tspackages/docs/search-box/v-model.spec.tspackages/docs/tsconfig.jsonpackages/search-box/index.tspackages/search-box/package.jsonpackages/search-box/scripts/plugin-utils.tspackages/search-box/shims-vue.d.tspackages/search-box/src/composables/use-dropdown.tspackages/search-box/src/index.tspackages/search-box/src/pc.vuepackages/search-box/src/shims-vue.d.tspackages/search-box/theme-saas/index.lesspackages/search-box/theme/index.lesspackages/search-box/tsconfig.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@packages/docs/.vitepress/config.mts`:
- Line 63: The favicon href can produce a double slash because env.VITE_BASE_URL
may end with a trailing '/', so update the head entry that builds the href (the
line using env.VITE_BASE_URL in packages/docs/.vitepress/config.mts) to strip
any trailing slash before appending '/favicon.ico' (e.g., call a
single-expression trim like .replace(/\/$/, '') on env.VITE_BASE_URL or fall
back to '/' appropriately) so the resulting href never contains a double slash.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 247f8057-60b2-4ed0-8545-3d54d4cfb059
📒 Files selected for processing (1)
packages/docs/.vitepress/config.mts
25d89a6 to
5e5c62e
Compare
5e5c62e to
4d3541e
Compare
Summary by CodeRabbit
New Features
Documentation
Tests
Chores