Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: partial support of server-client-mixed package + chore: add react-tweet demo #384

Merged

Conversation

hi-ogawa
Copy link
Owner

@hi-ogawa hi-ogawa commented Jun 10, 2024

todo

  • create a patch to make it an ideal external package
  • explore plugin level workaround
    • strip-off ?v= (this still causes dual modules when client is imported from both server (no ?v=) and client (with ?v=)
    • pre-bundling?
  • explore userland workaround
    • vendoring...?
    • package manager level workaround?
  • test
    • probably flaky to test react-tweet, so let's add fixture package to simulate the same.

@hi-ogawa
Copy link
Owner Author

hi-ogawa commented Jun 11, 2024

@hi-ogawa hi-ogawa force-pushed the chore-external-server-component-with-client-component-2 branch from 79a79b5 to 6391d04 Compare July 6, 2024 05:07
@hi-ogawa hi-ogawa force-pushed the chore-external-server-component-with-client-component-2 branch from d01eb84 to 4a1cd49 Compare July 15, 2024 08:03
Copy link

pkg-pr-new bot commented Jul 15, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

commit: 35a4491

@hiogawa/react-server

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/react-server@384

@hiogawa/react-server-next

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/react-server-next@384

@hiogawa/transforms

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/transforms@384

@hiogawa/vite-plugin-ssr-middleware

npm i https://pkg.pr.new/hi-ogawa/vite-plugins/@hiogawa/vite-plugin-ssr-middleware@384


templates

@hi-ogawa hi-ogawa changed the title chore: add react-tweet demo fix: partial support of server-client-mixed package + chore: add react-tweet demo Jul 15, 2024
Comment on lines 131 to 137
// when using external library's server component includes client reference,
// it will end up here with deps optimization hash `?v=` resolved by server module graph.
// this is not entirely free from double module issue,
// but it allows handling simple server-client-mixed package such as react-tweet.
if (!manager.buildType && id.includes("?v=")) {
id = id.split("?v=")[0]!;
}
Copy link
Owner Author

@hi-ogawa hi-ogawa Jul 15, 2024

Choose a reason for hiding this comment

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

I ranted a lot, but probably we can just merge this since we already have similar hack somewhere too and this doesn't make any worse...

@hi-ogawa hi-ogawa marked this pull request as ready for review July 15, 2024 08:46
@hi-ogawa hi-ogawa merged commit 8b254b4 into main Jul 15, 2024
9 checks passed
@hi-ogawa hi-ogawa deleted the chore-external-server-component-with-client-component-2 branch July 15, 2024 08:58
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.

1 participant