Skip to content

Conversation

@rivka-ungar
Copy link
Contributor

@rivka-ungar rivka-ungar commented Jan 1, 2026

User description

https://monday.monday.com/boards/3532714909/views/80492480/pulses/10887323505


PR Type

Enhancement


Description

  • Move Search component to independent @vibe/search package

  • Add useDebounceEvent hook to shared utilities package

  • Add useIsOverflowing and useResizeObserver hooks to typography

  • Update Search imports to use new package structure

  • Simplify Search component type definitions and dependencies


Diagram Walkthrough

flowchart LR
  A["Search Component"] -->|"moved to"| B["@vibe/search package"]
  C["useDebounceEvent hook"] -->|"moved to"| D["@vibe/shared"]
  E["useIsOverflowing hook"] -->|"added to"| F["@vibe/typography"]
  G["useResizeObserver hook"] -->|"added to"| F
  B -->|"imported by"| H["@vibe/core"]
  H -->|"uses"| I["Combobox component"]
Loading

File Walkthrough

Relevant files
Enhancement
9 files
Search.types.ts
Simplify Search component type imports                                     
+2/-4     
index.ts
Create package entry point                                                             
+1/-0     
useIsOverflowing.ts
Add overflow detection hook utility                                           
+54/-0   
useResizeObserver.ts
Add resize observer hook utility                                                 
+67/-0   
index.ts
Update Search export to new package                                           
+1/-1     
index.ts
Export new debounce event hook                                                     
+1/-0     
useDebounceEvent.ts
Add debounced event handler hook                                                 
+72/-0   
Search.tsx
Update imports to use shared utilities                                     
+1/-4     
Combobox.tsx
Update Search import to new package                                           
+1/-1     
Configuration changes
6 files
files.d.ts
Add SCSS module type declarations                                               
+4/-0     
package.json
Create package configuration for Search                                   
+73/-0   
rollup.config.mjs
Add rollup build configuration                                                     
+3/-0     
tsconfig.json
Add TypeScript configuration                                                         
+8/-0     
vitest.config.mjs
Add Vitest configuration                                                                 
+6/-0     
vitest.setup.mjs
Add test environment setup                                                             
+21/-0   
Formatting
1 files
Search.test.tsx
Minor test file formatting adjustment                                       
+1/-0     
Dependencies
1 files
package.json
Add Search package dependency                                                       
+1/-0     
Additional files
2 files
Search.module.scss [link]   
index.ts [link]   

@qodo-free-for-open-source-projects
Copy link
Contributor

qodo-free-for-open-source-projects bot commented Jan 1, 2026

PR Reviewer Guide 🔍

(Review updated until commit 254b818)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

State Update Logic

Lines 67-69 perform a state update outside of useEffect/useCallback, which could cause issues during render. This synchronous state update in the render phase may lead to unexpected behavior or warnings. Consider moving this logic into a useEffect hook.

if (initialStateValue !== previousValue.current && initialStateValue !== inputValue) {
  setValue(initialStateValue);
}
Missing Dependency

The useEffect hook has a disabled eslint rule for exhaustive-deps (line 65), but ref?.current is included in the dependency array (line 66). This is problematic because ref.current changes don't trigger re-renders. The effect should depend on ref itself, not ref?.current, or use a different approach to handle ref changes.

  // eslint-disable-next-line react-hooks/exhaustive-deps
}, [ref?.current, callback, debounceTime, debouncedCallback]);
Type Safety Reduction

The renderAction prop type was changed from a specific union type (IconButton | MenuButton) to a generic React.ReactElement. This reduces type safety and removes compile-time validation that only valid action components are passed. Consider keeping the more specific type or documenting the expected component types.

renderAction?: React.ReactElement;
/**

@@ -21,7 +19,7 @@ export interface SearchProps extends VibeComponentProps {
/**
* Renders an additional action button in the search input.
*/
renderAction?: React.ReactElement<typeof IconButton | typeof MenuButton>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to first split the Menubutton

@qodo-free-for-open-source-projects
Copy link
Contributor

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: bundle-size

Failed stage: Compare sizes [❌]

Failure summary:

The action failed due to multiple issues:

1. Build Error: A critical error occurred during the build process where the export
useDebounceEvent could not be found in packages/shared/dist/index.js (line 6093):
- ✘ [ERROR] No
matching export in "packages/shared/dist/index.js" for import "useDebounceEvent"
- This was
imported from packages/components/search/dist/Search/Search.js:1:117

2. Bundle Comparison Script Failure: The script scripts/bundle-check/compare-bundles.js failed
with error "No JSON array found in content" (lines 6660-6663). This script likely depends on the
output from the previous build step, which failed due to the missing export error.

The root cause is the missing useDebounceEvent export, which caused the build to fail with exit code
1, and subsequently the bundle comparison script couldn't find the expected JSON output to parse.

Relevant error logs:
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

126:  * [new branch]          feature/moro/disabled-legacy-editable-heading-text-selection -> origin/feature/moro/disabled-legacy-editable-heading-text-selection
127:  * [new branch]          feature/moshe/auto_scroll_after_back_on_store -> origin/feature/moshe/auto_scroll_after_back_on_store
128:  * [new branch]          feature/moshe/vulcan_upgrade -> origin/feature/moshe/vulcan_upgrade
129:  * [new branch]          feature/sergeyro/icon-deprecate-clickable -> origin/feature/sergeyro/icon-deprecate-clickable
130:  * [new branch]          feature/shanab/input-type-time -> origin/feature/shanab/input-type-time
131:  * [new branch]          fix-add-tslib               -> origin/fix-add-tslib
132:  * [new branch]          fix-chromatic-action        -> origin/fix-chromatic-action
133:  * [new branch]          fix-dialog-show-contextmenu-default-menu -> origin/fix-dialog-show-contextmenu-default-menu
134:  * [new branch]          fix-dropdown-spec           -> origin/fix-dropdown-spec
135:  * [new branch]          fix-showHideEvent-in-dialog-story -> origin/fix-showHideEvent-in-dialog-story
136:  * [new branch]          fix/combobox-spacings-9062326510 -> origin/fix/combobox-spacings-9062326510
137:  * [new branch]          fix/modal-focus             -> origin/fix/modal-focus
138:  * [new branch]          fix/orhal/playwright-install-performance-fix -> origin/fix/orhal/playwright-install-performance-fix
139:  * [new branch]          fix/yossi/test-focus-lock-esm -> origin/fix/yossi/test-focus-lock-esm
140:  * [new branch]          gh-pages                    -> origin/gh-pages
141:  * [new branch]          lint-error-fixes            -> origin/lint-error-fixes
142:  * [new branch]          master                      -> origin/master
...

2053:  �[2K�[1G�[2m$ node scripts/generate-lazy-icons.js�[22m
2054:  Generated lazy components and index.ts for 275 icons.
2055:  �[2K�[1G�[2m$ node scripts/generate-svg-index.js�[22m
2056:  Generated index.ts for SVG exports in ./src/svg
2057:  �[36m
2058:  �[1msrc/react/index.ts, src/lazy/index.ts, src/svg/index.ts, src/iconsMetaData.ts, src/types.ts�[22m → �[1mdist�[22m...�[39m
2059:  �[1m�[33m(!) Generated an empty chunk�[39m�[22m
2060:  types
2061:  �[32mcreated �[1mdist�[22m in �[1m45.5s�[22m�[39m
2062:  �[2K�[1GDone in 54.51s.
2063:  ##[endgroup]
2064:  Lerna (powered by Nx)   Successfully ran target build for 4 projects
2065:  Done in 84.74s.
2066:  ##[group]Run if [[ -n "$(git status --porcelain yarn.lock)" ]]; then
2067:  �[36;1mif [[ -n "$(git status --porcelain yarn.lock)" ]]; then�[0m
2068:  �[36;1m  echo "Error: yarn.lock has uncommitted changes. Please commit it."�[0m
2069:  �[36;1m  exit 1�[0m
...

6078:  packages/core/dist/src/utils/function-utils.js:1:6:
6079:  1 │ import"es-toolkit";function r(r){return Array.isArray(r)?r:[r]}func...
6080:  ╵       ~~~~~~~~~~~~
6081:  "sideEffects" is false in the enclosing "package.json" file:
6082:  node_modules/es-toolkit/package.json:325:2:
6083:  325 │   "sideEffects": false,
6084:  ╵   ~~~~~~~~~~~~~
6085:  ▲ [WARNING] Ignoring this import because "node_modules/es-toolkit/dist/index.mjs" was marked as having no side effects [ignored-bare-import]
6086:  packages/core/dist/src/utils/function-utils.js:1:6:
6087:  1 │ import"es-toolkit";function r(r){return Array.isArray(r)?r:[r]}func...
6088:  ╵       ~~~~~~~~~~~~
6089:  "sideEffects" is false in the enclosing "package.json" file:
6090:  node_modules/es-toolkit/package.json:325:2:
6091:  325 │   "sideEffects": false,
6092:  ╵   ~~~~~~~~~~~~~
6093:  ✘ [ERROR] No matching export in "packages/shared/dist/index.js" for import "useDebounceEvent"
6094:  packages/components/search/dist/Search/Search.js:1:117:
...

6637:  packages/core/dist/src/utils/function-utils.js:1:6:
6638:  1 │ import"es-toolkit";function r(r){return Array.isArray(r)?r:[r]}func...
6639:  ╵       ~~~~~~~~~~~~
6640:  "sideEffects" is false in the enclosing "package.json" file:
6641:  node_modules/es-toolkit/package.json:325:2:
6642:  325 │   "sideEffects": false,
6643:  ╵   ~~~~~~~~~~~~~
6644:  ▲ [WARNING] Ignoring this import because "packages/core/dist/src/utils/colors-vars-map.js" was marked as having no side effects [ignored-bare-import]
6645:  packages/core/dist/src/components/Table/Table/Table.js:1:296:
6646:  1 │ ...rops.js";import"../../../utils/colors-vars-map.js";import{getTab...
6647:  ╵                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
6648:  It was excluded from the "sideEffects" array in the enclosing "package.json" file:
6649:  packages/core/package.json:278:2:
6650:  278 │   "sideEffects": [
6651:  ╵   ~~~~~~~~~~~~~
6652:  ##[error]Process completed with exit code 1.
6653:  ##[group]Run node scripts/bundle-check/compare-bundles.js
6654:  �[36;1mnode scripts/bundle-check/compare-bundles.js�[0m
6655:  shell: /usr/bin/bash -e {0}
6656:  env:
6657:  NPM_CONFIG_USERCONFIG: /home/runner/work/_temp/.npmrc
6658:  NODE_AUTH_TOKEN: XXXXX-XXXXX-XXXXX-XXXXX
6659:  ##[endgroup]
6660:  /home/runner/work/vibe/vibe/scripts/bundle-check/compare-bundles.js:9
6661:  throw new Error("No JSON array found in content");
6662:  ^
6663:  Error: No JSON array found in content
6664:  at extractJson (/home/runner/work/vibe/vibe/scripts/bundle-check/compare-bundles.js:9:11)
6665:  at Object.<anonymous> (/home/runner/work/vibe/vibe/scripts/bundle-check/compare-bundles.js:18:25)
6666:  at Module._compile (node:internal/modules/cjs/loader:1369:14)
6667:  at Module._extensions..js (node:internal/modules/cjs/loader:1427:10)
6668:  at Module.load (node:internal/modules/cjs/loader:1206:32)
6669:  at Module._load (node:internal/modules/cjs/loader:1022:12)
6670:  at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12)
6671:  at node:internal/main/run_main_module:28:49
6672:  Node.js v20.12.2
6673:  ##[error]Process completed with exit code 1.
6674:  ##[group]Run actions/upload-artifact@v4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants