-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat(vue): support SSR #468
Conversation
7f5bbc6
to
8236c27
Compare
095b690
to
e984dec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot here, but looks like it works as expected. I tested out the Nuxt app and everything was rendering (some of the components are goofy, but that's just the components, not the SSR implementation). I didn't double-check a client-side Vue app.
@@ -66,8 +66,12 @@ export const config: Config = { | |||
hydrateModule: 'component-library/hydrate' | |||
}), | |||
vueOutputTarget({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have two Vue output target configs like we do for React?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React and Vue projects are setup differently here. We have two output targets for React because we directly compile the components into their represented projects. With Vue we have setup an example project that just re-exports the components, and potentially more Vue related modules. That example project is imported by Nuxt and the Vite project. A bit different setup but it is actually how we see projects being organised.
FWIW I raised PR adding an example app using Vite + Vue, see #499. For that I've added an e2e test for this. For all these environments we now run a WebdriverIO tests that verifies that Stencil is properly able to hydrate the component, e.g. interacting with an input component with SSR support or without. While these tests are very rudimental, they are a start a least 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it was part of the basic Nuxt setup. I can remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, added some comments but they're not blockers
* chore: re-org package structure * update deps in next project * lerna change * add back missing components dir * fix tests * fix react tests * fix types * run headless * ignore React type mismatch * build nextjs project alongside other projects * update unit test * fix(react): don't populate non-primitive objects (#498) * chore: re-org package structure * fix(react): don't populate non-primitive objects * test(vue): add playground for Vite+Vue (#499) * test(vue): add playground for Vite+Vue * ignore type issue
2c8d1e9
to
4ee8c6d
Compare
Pull request checklist
Please check if your PR fulfills the following requirements:
npm run build
) was run locally for affected output targetsnpm test
) were run locally and passednpm run prettier
) was run locally and passedPull request type
Please check the type of change your PR introduces:
What is the current behavior?
Currently the Vue Output Target would not support SSR.
What is the new behavior?
This patch adds support for it. Furthermore:
@stencil/vue-output-target
as dependency to access runtime files (similar to React)hydrateModule
property to the output target that allows users to enable SSRDoes this introduce a breaking change?
Other information