feat: Add plugin option for custom outDir #591

waigel · 2024-07-31T00:38:49Z

Corrected the build process to honor the user-defined outDir setting in the Vite configuration.

…igurable-out-dir

hi-ogawa

Hey, thanks for taking a look. It wasn't a priority but obviously hard-coding dist/ everywhere is not great and I was hoping I can clean this up someday.

If you have a concrete motivation to change outDir other than dist, then I would like to know the reason as it helps setting the priority.

packages/react-server/src/entry/ssr.tsx

packages/react-server/src/plugin/index.ts

hi-ogawa · 2024-07-31T02:30:28Z

On second thought, I think trying to reuse Vite config's outDir as user option might be conceptually difficult. What Sveltekit does is to expose outDir option (default .svelte-kit) as svelte.config.js opiton https://kit.svelte.dev/docs/configuration#outdir.

Probably it makes sense to not support outDir as vite config level, but only support it as plugin option i.e.

// vite.config.ts
export default defineConfig({
  plugins: [
    vitePluginReactServer({
      outDir: "custom-dir", // here okay
    })
  ],
  build: {
    outDir: "...", // not supported here
  }
})

This probably sets the expectation better and also code becomes less confusing?

waigel · 2024-07-31T10:39:03Z

Hey, thank you so much for your detailed feedback and for taking the time to provide tips for improvement. I really appreciate your insights!

Firstly, the reason for customizing the outDir is primarily due to internal considerations related to existing workflows and build caches. While this is not a high-priority issue, I agree that it should be more configurable, as you have mentioned.

I would be happy to assist with this feature and will carefully review your comments to revise the draft accordingly.

Additionally, I believe you are right that a custom plugin-scoped outDir makes more sense and would be clearer to communicate. I will continue the approach.

hi-ogawa

Thanks for the follow up! It looks about right, but I think there are a few missing on react-server-next adapter side

vite-plugins/packages/react-server-next/src/vite/adapters/vercel/build.ts

Lines 41 to 42 in f85a833

    
           export async function build({ runtime }: { runtime: VercelRuntime }) { 
        
             const buildDir = join(process.cwd(), "dist");

You can also add your example on CI. I wasn't sure if adding an entire example is worth it but for now it should be fine. You can add it to test-react-server-others job around here:

vite-plugins/.github/workflows/ci.yml

Lines 48 to 50 in f85a833

    
           test-react-server-others: 
        
             runs-on: ubuntu-latest 
        
             steps:

I just approved CI run on this PR, so you can check it.

packages/react-server/src/plugin/index.ts

packages/react-server/src/entry/ssr.tsx

waigel · 2024-08-02T19:10:12Z

I’ve addressed your feedback and implemented the changes. I’m confident I’ve covered all the hard-coded ‘dist’ directories, and the pipeline should now pass smoothly, even with the new e2e tests.

hi-ogawa · 2024-08-03T02:48:23Z

Awesome, code looks good to me! I might tweak tests for my own taste, but we should be able to merge this soon. Thanks a lot.

If you want to test this out early, you can grab packages built from this PR here https://github.com/hi-ogawa/vite-plugins/runs/28288606525

You can use it like "@hiogawa/react-server": "https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/react-server@af4f6b6" in your package dependencies.

hi-ogawa · 2024-08-03T04:10:09Z

packages/react-server/examples/custom-out-dir/app/layout.tsx

@@ -0,0 +1,12 @@
+import type React from "react";
+
+export default function Layout(props: React.PropsWithChildren) {


We don't have default root layout (I think next.js has it?), so we need to add one here. Otherwise, ssr/hydration is failing since they expect <html> and <body> are rendered.

packages/react-server/examples/custom-out-dir/app/page.tsx

packages/react-server/src/entry/ssr.tsx

hi-ogawa · 2024-08-03T05:48:55Z

Merging. Thanks for the contribution!

waigel added 3 commits July 31, 2024 02:36

fix: Ensure Vite respects custom outDir in build configuration

86ce9ec

fix: Ensure Vite respects custom outDir in build configuration

b76b2a3

Merge branch 'feat-configurable-out-dir-test-pipeline' into feat-conf…

a0b5e2e

…igurable-out-dir

waigel marked this pull request as draft July 31, 2024 01:44

hi-ogawa reviewed Jul 31, 2024

View reviewed changes

packages/react-server/src/entry/ssr.tsx Outdated Show resolved Hide resolved

hi-ogawa reviewed Jul 31, 2024

View reviewed changes

packages/react-server/src/plugin/index.ts Outdated Show resolved Hide resolved

waigel added 2 commits August 1, 2024 21:45

chore: RSC import with virtual-modules

e842c87

chore: Use outDir in react-server-next plugin

9025ecf

hi-ogawa reviewed Aug 2, 2024

View reviewed changes

packages/react-server/src/plugin/index.ts Outdated Show resolved Hide resolved

packages/react-server/src/entry/ssr.tsx Outdated Show resolved Hide resolved

waigel added 2 commits August 2, 2024 13:20

chore: Directly use virtual module and extend next with outDir

45e272f

chore: Execute e2e custom-out-dir test in CI

e05fe41

waigel changed the title ~~fix: Ensure Vite respects custom outDir in build configuration~~ feat: Add plugin option for custom outDir Aug 2, 2024

waigel marked this pull request as ready for review August 2, 2024 19:00

Merge branch 'main' into feat-configurable-out-dir

af4f6b6

hi-ogawa added 6 commits August 3, 2024 11:54

refactor: rename to 'virtual:react-server-build' + remove typing

4a58dc7

chore: revert format

75c525c

chore: simpify example and e2e

b213b9c

chore: simplify gitignore

1444172

test: test import.meta.url

99acc2a

chore: cleanup

d69f219

hi-ogawa approved these changes Aug 3, 2024

View reviewed changes

hi-ogawa merged commit 614b597 into hi-ogawa:main Aug 3, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add plugin option for custom outDir #591

feat: Add plugin option for custom outDir #591

waigel commented Jul 31, 2024

hi-ogawa left a comment

hi-ogawa commented Jul 31, 2024

waigel commented Jul 31, 2024

hi-ogawa left a comment •

edited

Loading

waigel commented Aug 2, 2024

hi-ogawa commented Aug 3, 2024 •

edited

Loading

hi-ogawa Aug 3, 2024

hi-ogawa commented Aug 3, 2024

	export async function build({ runtime }: { runtime: VercelRuntime }) {
	const buildDir = join(process.cwd(), "dist");

		@@ -0,0 +1,12 @@
		import type React from "react";

		export default function Layout(props: React.PropsWithChildren) {

feat: Add plugin option for custom outDir #591

feat: Add plugin option for custom outDir #591

Conversation

waigel commented Jul 31, 2024

hi-ogawa left a comment

Choose a reason for hiding this comment

hi-ogawa commented Jul 31, 2024

waigel commented Jul 31, 2024

hi-ogawa left a comment • edited Loading

Choose a reason for hiding this comment

waigel commented Aug 2, 2024

hi-ogawa commented Aug 3, 2024 • edited Loading

hi-ogawa Aug 3, 2024

Choose a reason for hiding this comment

hi-ogawa commented Aug 3, 2024

hi-ogawa left a comment •

edited

Loading

hi-ogawa commented Aug 3, 2024 •

edited

Loading