Skip to content

Conversation

@mpeg
Copy link
Contributor

@mpeg mpeg commented Nov 24, 2025

  • Everywhere that checks the isSSRBuild flag, we add a matching this.environment check for compatibility with the vite env api
  • Enable sharedDuringBuild flag on both plugins, so one instance is shared on an app build
  • Add configEnvironment hook, a bit of duplication here with the config hook for ssr builds, but intent is clearer this way
  • Add a buildApp hook which by default builds in order, first ssr then client env (when doing a linked build). If both envs are not set it assumes vite is on legacy mode and does nothing to avoid extra builds.
  • buildApp can be overriden by the end user through builder.buildApp in the vite config, this matches the behaviour of other plugins such as the cloudflare vite plugin

On Vite 6 and under, the new hooks are ignored and do nothing.

A minimal vite config to work with the new APIs would look like this

export default defineConfig({
	environments: {
		ssr: {
			build: {
				rollupOptions: {
					input: "src/entry.ts",
				},
			},
		},
		client: {
		},
	},
	plugins: [marko()]
});

and build with vite build --app
for the legacy APIs, you can still specify the entry point on the --ssr command like vite build --ssr src/entry.ts && vite build

mpeg added 3 commits November 21, 2025 20:52
similar to what cloudflare vite plugin does here https://github.com/cloudflare/workers-sdk/blob/24765d1c1e5cf8d48ea719440a6f74a6b5d90402/packages/vite-plugin-cloudflare/src/plugins/config.ts#L85

we do it in buildApp instead of config which runs later, but it retains backwards compatibility with the legacy vite apis
@changeset-bot
Copy link

changeset-bot bot commented Nov 24, 2025

🦋 Changeset detected

Latest commit: 4789c74

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@marko/vite Minor

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 24, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@coderabbitai
Copy link

coderabbitai bot commented Nov 24, 2025

Walkthrough

Adds build orchestration and SSR-aware behavior to the Vite plugin: introduces a pre-plugin buildApp(builder) and marks pre/post plugins with sharedDuringBuild: true; adds configEnvironment(name, options) to the plugin API. Unifies SSR detection to this?.environment?.name === "ssr". Adjusts config.build.emptyOutDir handling for linked and SSR builds, extends resolve.noExternal to include .marko, enables commonjsOptions.esmExternals, defaults SSR Rollup chunkFileNames when missing, and updates server/client manifest and asset loading. Adds a changeset for a minor version bump.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add vite environments api compatibility' clearly and concisely summarizes the main change: adding support for Vite's environments API.
Description check ✅ Passed The description is directly related to the changeset, explaining the implementation details of Vite environments API compatibility including hook additions, environment checks, and configuration examples.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

@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: 1

🧹 Nitpick comments (2)
src/index.ts (2)

432-435: Potential duplication with configEnvironment hook.

This logic setting emptyOutDir = false for non-SSR builds is duplicated in the configEnvironment hook (lines 531-534). While this may be intentional for backward compatibility with legacy Vite (before environments API), it could lead to confusion about which hook controls this setting.

Consider adding a comment explaining that this is for legacy Vite compatibility, or refactoring to centralize the emptyOutDir logic:

 if (!isSSRBuild) {
+  // Legacy Vite: prevent client build from clearing SSR output
   config.build ??= {};
   config.build.emptyOutDir = false;
 }

493-537: Consider extracting duplicated configuration logic.

The configEnvironment hook duplicates substantial logic from the config hook:

  • noExternal handling (lines 496-508 vs 395-408)
  • chunkFileNames configuration (lines 513-521 vs 410-419)
  • esmExternals setup (lines 522-530 vs 421-430)

While this duplication supports both legacy and environments API modes, it increases maintenance burden.

Consider extracting the shared logic into helper functions:

function configureNoExternal(noExternal: any): any {
  if (noExternal === true) return noExternal;
  const noExternalReg = /\.marko$/;
  if (noExternal) {
    return Array.isArray(noExternal) 
      ? [...noExternal, noExternalReg]
      : [noExternal, noExternalReg];
  }
  return noExternalReg;
}

function configureSSRBuildOptions(options: any, rootResolveFile: string) {
  options.build ??= {};
  if (!options.build?.rollupOptions?.output) {
    options.build.rollupOptions ??= {};
    options.build.rollupOptions.output = {
      chunkFileNames: `[name]-[hash].js`,
    };
  }
  if (!options.build?.commonjsOptions?.esmExternals) {
    options.build.commonjsOptions ??= {};
    options.build.commonjsOptions.esmExternals = (id) =>
      !isCJSModule(id, rootResolveFile);
  }
}

Then use these helpers in both hooks.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c84cc4 and 8ff20a3.

📒 Files selected for processing (1)
  • src/index.ts (7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (1)
src/resolve.ts (1)
  • isCJSModule (10-51)
🔇 Additional comments (4)
src/index.ts (4)

603-635: LGTM! Proper dual-mode SSR detection.

The condition correctly checks both isSSRBuild (legacy mode) and this?.environment?.name === "ssr" (environments API mode), ensuring backward compatibility while supporting the new Vite environments API.


636-650: LGTM! Correct dual-mode non-SSR detection.

The negated condition properly identifies non-SSR builds in both legacy mode (!isSSRBuild) and environments API mode (this?.environment?.name !== "ssr"), correctly loading server assets during client builds.


983-987: LGTM! Consistent plugin sharing configuration.

Adding sharedDuringBuild: true to the post plugin matches the pre plugin configuration, correctly enabling plugin instance sharing during environment-based builds as described in the PR objectives.


1000-1040: LGTM! Consistent dual-mode SSR detection in bundle generation.

The condition properly checks both modes for SSR context when handling server manifests and assets during bundle generation, maintaining consistency with the SSR detection pattern used throughout the file.

Copy link

@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: 0

🧹 Nitpick comments (1)
.changeset/frank-wings-share.md (1)

1-5: Minor: Capitalize "Vite" in changeset description.

The changeset structure is valid and the minor version bump is appropriate for this feature addition. However, the description should capitalize "Vite" for brand consistency.

---
"@marko/vite": minor
---

-compatibility with the vite environments api
+Adds vite environments API compatibility

This slight rewording also clarifies the purpose and follows common changelog conventions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ff20a3 and 57340cf.

📒 Files selected for processing (1)
  • .changeset/frank-wings-share.md (1 hunks)

Copy link

@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: 0

🧹 Nitpick comments (1)
src/index.ts (1)

432-435: emptyOutDir and env‑specific config are consistent, but SSR logic is duplicated

The combination of:

  • Line 432–435: forcing config.build.emptyOutDir = false for non‑SSR linked builds, and
  • Lines 493–539: per‑environment configEnvironment that sets emptyOutDir = true for ssr and false for other envs when linked, while mirroring the .marko noExternal, chunkFileNames, and esmExternals logic

produces the intended behavior for both legacy linked builds and the environments API. The main downside is that the SSR config (noExternal/chunkFileNames/esmExternals/emptyOutDir) is now duplicated between config and configEnvironment, which increases drift risk if one side is tweaked later.

Consider extracting a small helper (e.g. applySSRBuildConfig(targetBuild, targetResolve, rootResolveFile) or similar) that both config and configEnvironment("ssr", ...) call, so changes stay in sync while keeping the env‑specific emptyOutDir override. This is non‑blocking but would make future maintenance safer.

Also applies to: 493-539

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 57340cf and e9b5dc5.

📒 Files selected for processing (1)
  • src/index.ts (7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mpeg
Repo: marko-js/vite PR: 236
File: src/index.ts:207-222
Timestamp: 2025-11-24T13:52:04.055Z
Learning: In Vite's buildApp hook (available in Vite 6+), the builder parameter has a config property that can contain builder.buildApp. The property path builder.config.builder.buildApp is valid for checking if a user has provided a custom buildApp override in their Vite configuration to control build order and avoid conflicts between multiple plugins.
📚 Learning: 2025-11-24T13:52:04.055Z
Learnt from: mpeg
Repo: marko-js/vite PR: 236
File: src/index.ts:207-222
Timestamp: 2025-11-24T13:52:04.055Z
Learning: In Vite's buildApp hook (available in Vite 6+), the builder parameter has a config property that can contain builder.buildApp. The property path builder.config.builder.buildApp is valid for checking if a user has provided a custom buildApp override in their Vite configuration to control build order and avoid conflicts between multiple plugins.

Applied to files:

  • src/index.ts
🧬 Code graph analysis (1)
src/index.ts (2)
src/__tests__/fixtures/isomorphic-runtime-base-path/test.config.ts (2)
  • ssr (17-17)
  • options (25-27)
src/resolve.ts (1)
  • isCJSModule (10-51)
🔇 Additional comments (3)
src/index.ts (3)

206-222: buildApp hook wiring and override behavior look correct

The sharedDuringBuild: true flag plus buildApp(builder) implementation align with Vite’s environments API: you only take over when both ssr and client environments exist, preserve legacy behavior when they don’t, and correctly defer to a user‑provided builder.config.builder.buildApp to avoid double orchestration. The linked‑only ssr build followed by an unconditional client build matches the documented “SSR then client for linked builds” semantics. Based on learnings, the builder.config.builder.buildApp override check is the right shape.


607-635: SSR environment detection via this.environment is coherent across hooks

Using isSSRBuild || this?.environment?.name === "ssr" in options (Line 607) and generateBundle (Line 1013), and the complementary guard in buildStart (Lines 639–645), cleanly unifies legacy CLI SSR builds (isSSRBuild from config.build.ssr) with the new environments API (this.environment.name === "ssr"). This ensures:

  • The SSR manifest is initialized exactly once per SSR environment.
  • Browser/client builds (legacy or env‑based) correctly consume the persisted SSR manifest and eagerly load SSR assets.
  • The same SSR vs browser branch is taken in generateBundle regardless of whether SSR is driven by --ssr or by the env API.

The optional chaining on this?.environment is conservative but harmless given plugin hook contexts. Overall the flow looks correct and consistent with the new buildApp/configEnvironment behavior.

Also applies to: 639-645, 1013-1043


986-990: Post plugin sharedDuringBuild flag matches pre plugin behavior

Marking the marko-vite:post plugin as sharedDuringBuild: true is important so its state (notably access to serverManifest written by the pre plugin and shared closure state) is reused across SSR/client environment builds within a single app build. This keeps generateBundle’s SSR and browser passes aligned with the pre plugin’s buildApp orchestration and env‑aware options logic. Comment tweak is fine as well.

@mpeg
Copy link
Contributor Author

mpeg commented Nov 25, 2025

I've fixed the unnecessary optional chaining, since builder will always be defined when the buildApp hook is called, and this should always be defined in the plugin hooks

Also added a linked guard around the emptyOutDir default, so it only defaults to not emptying it on client builds when part of a linked build.

Copy link

@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: 1

🧹 Nitpick comments (1)
src/index.ts (1)

493-539: Consider extracting shared SSR build config into a reusable helper.

The configEnvironment("ssr", options) branch largely duplicates the SSR setup done earlier in config(...) (adding .marko to noExternal, overriding chunkFileNames, and wiring commonjsOptions.esmExternals). To avoid the two paths drifting over time, you could factor this into a small helper (e.g. applySSRBuildDefaults(configLike) and applySSRNoExternal(configLike)), and call it from both config and configEnvironment. This keeps legacy --ssr and environment‑API SSR behavior in lockstep.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9b5dc5 and 4789c74.

📒 Files selected for processing (1)
  • src/index.ts (7 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mpeg
Repo: marko-js/vite PR: 236
File: src/index.ts:207-222
Timestamp: 2025-11-24T13:52:04.055Z
Learning: In Vite's buildApp hook (available in Vite 6+), the builder parameter has a config property that can contain builder.buildApp. The property path builder.config.builder.buildApp is valid for checking if a user has provided a custom buildApp override in their Vite configuration to control build order and avoid conflicts between multiple plugins.
📚 Learning: 2025-11-24T13:52:04.055Z
Learnt from: mpeg
Repo: marko-js/vite PR: 236
File: src/index.ts:207-222
Timestamp: 2025-11-24T13:52:04.055Z
Learning: In Vite's buildApp hook (available in Vite 6+), the builder parameter has a config property that can contain builder.buildApp. The property path builder.config.builder.buildApp is valid for checking if a user has provided a custom buildApp override in their Vite configuration to control build order and avoid conflicts between multiple plugins.

Applied to files:

  • src/index.ts
🧬 Code graph analysis (1)
src/index.ts (1)
src/resolve.ts (1)
  • isCJSModule (10-51)
🔇 Additional comments (3)
src/index.ts (3)

432-435: Double‑check the impact of forcing emptyOutDir = false for all non‑SSR linked builds.

With linked defaulting to true, this will also apply to plain browser builds (e.g. vite build without a preceding SSR build), meaning outDir is no longer cleaned by default and old artifacts may accumulate. That’s probably fine for the linked SSR+client workflow, but it is a behavior change vs Vite defaults; worth confirming this is intentional and reflected in docs/expectations.


605-637: Unified SSR detection in options/buildStart looks correct.

Using isSSRBuild || this.environment?.name === "ssr" in options and the mirrored negation in buildStart ensures serverManifest is created for SSR builds (legacy and environment‑API) and loaded only for the corresponding non‑SSR/browser build. The linked && isBuild guards also prevent serverManifest! from being touched in dev/test. This is a clean way to support both code paths without changing the existing manifest flow.

Also applies to: 639-645


986-990: Post‑plugin sharedDuringBuild and SSR branch gating align with the pre‑plugin changes.

Marking the post plugin as sharedDuringBuild: true matches the pre plugin and ensures a single shared serverManifest/store is used across environments. The updated condition if (isSSRBuild || this.environment?.name === "ssr") in generateBundle correctly confines manifest collection and persistence to the server build for both legacy --ssr and the new ssr environment, while the else‑branch continues to handle the browser manifest case. No issues spotted here.

Also applies to: 1013-1043

@DylanPiercey DylanPiercey merged commit c49203b into marko-js:main Nov 26, 2025
9 checks passed
@github-actions github-actions bot mentioned this pull request Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants