Skip to content

Revert no bundling approach#1479

Open
flevi29 wants to merge 1 commit into
mainfrom
revert-no-bundle-approach
Open

Revert no bundling approach#1479
flevi29 wants to merge 1 commit into
mainfrom
revert-no-bundle-approach

Conversation

@flevi29
Copy link
Copy Markdown
Collaborator

@flevi29 flevi29 commented Apr 25, 2026

Pull Request

Related issue

Fixes #1478

What does this PR do?

  • ...

PR checklist

Please check if your PR fulfills the following requirements:

  • Did you use any AI tool while implementing this PR (code, tests, docs, etc.)? If yes, disclose it in the PR description and describe what it was used for. AI usage is allowed when it is disclosed.
  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Summary by CodeRabbit

  • Chores
    • Optimized build pipeline with improved compilation process
    • Enhanced TypeScript configuration for type declaration handling
    • Improved source map generation for debugging support

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 25, 2026

⚠️ No Changeset found

Latest commit: c2096ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

📝 Walkthrough

Walkthrough

The build pipeline for autocomplete-client and instant-meilisearch packages is updated to run Vite bundling before TypeScript compilation. TypeScript configurations are modified to emit only type declarations while source maps are disabled in tsc, delegating the primary build work to Vite with proper library mode and ES module output configuration.

Changes

Cohort / File(s) Summary
Build Scripts
packages/autocomplete-client/package.json, packages/instant-meilisearch/package.json
Build script updated from tsc to vite build && tsc, introducing Vite bundling as the first build step.
TypeScript Configuration
packages/autocomplete-client/tsconfig.json, packages/instant-meilisearch/tsconfig.json
Added emitDeclarationOnly: true to skip JavaScript generation, removed sourceMap: true, and updated glob patterns to use ./src/... prefix.
Vite Configuration
packages/autocomplete-client/vite.config.ts, packages/instant-meilisearch/vite.config.ts
Added Vite build configuration that loads root tsconfig target, enables sourcemaps, and configures library mode output with ES format and src/index.ts entry point.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A bundler's dance, Vite takes the lead,
While TypeScript declares what we need,
Two packages now build with fresh might,
No-bundler worlds shining so bright! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Revert no bundling approach' directly and specifically describes the primary change—reverting a prior decision to remove bundling and restoring bundle generation to fix importmap/no-bundler environments.
Linked Issues check ✅ Passed The PR addresses issue #1478 by reverting the no-bundling approach: changes to Vite configs add explicit library bundling (sourcemaps, library mode, ES format), tsconfigs switch to declaration-only emission, and package.json scripts include vite build before tsc. These changes restore bundled output to fix importmap/no-bundler compatibility.
Out of Scope Changes check ✅ Passed All changes are in-scope: modifications to tsconfig.json, vite.config.ts, and package.json build scripts across autocomplete-client and instant-meilisearch packages are directly related to re-enabling bundling as required by issue #1478.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch revert-no-bundle-approach

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/autocomplete-client/package.json`:
- Line 7: The package lists `@meilisearch/instant-meilisearch` as a regular
dependency which causes Vite library mode to inline its source into the built
bundle; either move "@meilisearch/instant-meilisearch" from dependencies to
peerDependencies in packages/autocomplete-client/package.json or modify the
library build config (vite.config.ts) for the autocomplete-client target to add
rollupOptions.external including "@meilisearch/instant-meilisearch" so Rollup
treats it as external; update package.json and/or vite.config.ts (look for the
build script and the vite config entry for this package and the
rollupOptions.external setting) and publish with the dependency externalized to
avoid bundling duplicate copies.

In `@packages/autocomplete-client/vite.config.ts`:
- Around line 6-14: The build currently inlines dependencies because Vite only
externals peerDependencies; update the build configuration inside the existing
build block (the same block containing lib.entry and fileName) to add
rollupOptions.external that explicitly externalizes
'@meilisearch/instant-meilisearch' and 'meilisearch' (use exact names and/or a
regex to catch subpath imports) so the Rollup bundler will not inline their
source into dist/index.js; ensure you update the build.rollupOptions.external
array in vite.config.ts accordingly.
🪄 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: bfb5e4df-9f2d-4419-9698-af0df0e0a5f5

📥 Commits

Reviewing files that changed from the base of the PR and between 925b93f and c2096ad.

📒 Files selected for processing (6)
  • packages/autocomplete-client/package.json
  • packages/autocomplete-client/tsconfig.json
  • packages/autocomplete-client/vite.config.ts
  • packages/instant-meilisearch/package.json
  • packages/instant-meilisearch/tsconfig.json
  • packages/instant-meilisearch/vite.config.ts

"homepage": "https://github.com/meilisearch/meilisearch-js-plugins/tree/main/packages/autocomplete-client",
"scripts": {
"build": "tsc"
"build": "vite build && tsc"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Build pipeline OK; see vite.config.ts for an externals concern that affects this published bundle.

The script itself is correct, but autocomplete-client has @meilisearch/instant-meilisearch as a regular dependency. Vite library mode only externalizes peerDependencies by default, so without explicit rollupOptions.external the workspace instant-meilisearch source (its exports resolves to ./src/index.ts) will be inlined into this package's dist/index.js. Consumers installing both packages would ship two copies. See the vite.config.ts comment for the suggested fix.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/autocomplete-client/package.json` at line 7, The package lists
`@meilisearch/instant-meilisearch` as a regular dependency which causes Vite
library mode to inline its source into the built bundle; either move
"@meilisearch/instant-meilisearch" from dependencies to peerDependencies in
packages/autocomplete-client/package.json or modify the library build config
(vite.config.ts) for the autocomplete-client target to add
rollupOptions.external including "@meilisearch/instant-meilisearch" so Rollup
treats it as external; update package.json and/or vite.config.ts (look for the
build script and the vite config entry for this package and the
rollupOptions.external setting) and publish with the dependency externalized to
avoid bundling duplicate copies.

Comment on lines +6 to +14
build: {
sourcemap: true,
target: tsconfig.compilerOptions.target,
lib: {
entry: 'src/index.ts',
formats: ['es'],
fileName: 'index',
},
},
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Vite library mode externalize regular dependencies default behavior

💡 Result:

In Vite's library mode (enabled via build.lib), regular dependencies are bundled into the library output by default. Vite does not automatically externalize them. To externalize dependencies (prevent bundling), explicitly configure build.rollupOptions.external with the dependency names, typically loaded from package.json dependencies: import pkg from './package.json' export default defineConfig({ build: { lib: { /* ... */ }, rollupOptions: { external: Object.keys(pkg.dependencies || {}) } } }) Official Vite docs recommend manually externalizing deps you do not want bundled, e.g., React/Vue. Examples in docs show empty { rollupOptions: {} } or explicit externals like ['vue']. Community discussions confirm no auto-externalization; plugins like vite-plugin-externalize-deps exist as workarounds. Source code shows rollupOptions.external used directly if provided, otherwise undefined (bundles deps). This behavior persists as of 2026.

Citations:


🏁 Script executed:

# Check the package.json to see dependency classification
cat packages/autocomplete-client/package.json | grep -A 20 '"dependencies"' | head -15

Repository: meilisearch/meilisearch-js-plugins

Length of output: 217


🏁 Script executed:

# Check the current vite.config.ts file
cat packages/autocomplete-client/vite.config.ts

Repository: meilisearch/meilisearch-js-plugins

Length of output: 533


Externalize @meilisearch/instant-meilisearch (and meilisearch) explicitly.

In library mode Vite externalizes only peerDependencies. Since @meilisearch/instant-meilisearch is listed under dependencies and its workspace exports map resolves . to ./src/index.ts, Vite will inline its entire source tree into this package's dist/index.js. End users that also install @meilisearch/instant-meilisearch (which is the typical case) would then get two divergent copies and a larger bundle. Declaring it (and the meilisearch peer, for safety/regex coverage) as external avoids that.

♻️ Proposed fix
 export default defineConfig({
   build: {
     sourcemap: true,
     target: tsconfig.compilerOptions.target,
     lib: {
       entry: 'src/index.ts',
       formats: ['es'],
       fileName: 'index',
     },
+    rollupOptions: {
+      external: [/^@meilisearch\/instant-meilisearch(\/|$)/, /^meilisearch(\/|$)/],
+    },
   },
📝 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.

Suggested change
build: {
sourcemap: true,
target: tsconfig.compilerOptions.target,
lib: {
entry: 'src/index.ts',
formats: ['es'],
fileName: 'index',
},
},
build: {
sourcemap: true,
target: tsconfig.compilerOptions.target,
lib: {
entry: 'src/index.ts',
formats: ['es'],
fileName: 'index',
},
rollupOptions: {
external: [/^@meilisearch\/instant-meilisearch(\/|$)/, /^meilisearch(\/|$)/],
},
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/autocomplete-client/vite.config.ts` around lines 6 - 14, The build
currently inlines dependencies because Vite only externals peerDependencies;
update the build configuration inside the existing build block (the same block
containing lib.entry and fileName) to add rollupOptions.external that explicitly
externalizes '@meilisearch/instant-meilisearch' and 'meilisearch' (use exact
names and/or a regex to catch subpath imports) so the Rollup bundler will not
inline their source into dist/index.js; ensure you update the
build.rollupOptions.external array in vite.config.ts accordingly.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@flevi29 WDYT?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

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.

v0.31.x breaks importmap / no-bundler environments

2 participants