-
Notifications
You must be signed in to change notification settings - Fork 117
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(react): Next.js Support #445
Conversation
661ae44
to
cbceabb
Compare
cbceabb
to
f3a59a8
Compare
PR with additional documentation can be found in ionic-team/stencil-site#1458 |
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. This is a good one to get some people testing before we merge all the way back to main, which I think is the plan anyway
@christian-bromann @tanner-reits We would happily test the new output target on very large NextJS project. Do you have a dev version we could use? |
That's awesome! I will share a dev version once I got ionic-team/stencil#5892 embedded in this. |
great news! do you have any further info on what exactly Next JS support looks like. Im currently using a Provider pattern so though I render all Ionic components on the client (with 'use client'). Regarding I am able to load the context through the root layout so benefit from everything Next JS app router has to offer. Borrowed the pattern from Redux (which works really well with Ionic components and Next JS and yay im controlling my mobile app state with Redux Toolkit) . I guess some components will have to be rendered on the client i.e. those with interactivity. I anticipate thats quite a lot of components. I also anticipate that the list of components that can be rendered server side will be well defined and limited. Is there a discussion on this which can be used as the foundation for docs later on, or at least to manage expectations during testing now? |
With this patch the react output target will allow you to create a Next.js framework wrapper which enables the usage of Stencil components within Next.js applications including support for SSR, so that you can use Stencil components also on the server side.
I think this PR thread is the best for posting comments and providing feedback on this development. @mayerraphael I've just created this dev build: |
@christian-bromann Thanks for the dev Version. We have problems getting it running. Dist Custom Elements dirWe had to add a dir to the dist-custom-elements output target options otherwise the wrong import paths have been generated. {
type: 'dist-custom-elements',
dir: 'dist/components', // added
}, It looks like Stencil uses dist/components as default and the Output Target uses /components as default, see: https://github.com/ionic-team/stencil-ds-output-targets/blob/next/packages/react-output-target/src/react-output-target/react-output-target.ts#L34 Event with underscore got brokenEvents that had an underscore prefix got broken inside the generated components.ts file. But this may be on our side. We removed those prefixes. Server render errors
Error: Objects are not valid as a React child (found: [object Promise]). If you meant to render a collection of children, use an array instead.
https://reactjs.org/docs/error-decoder.html?invariant=31&args[]=%5Bobject%20Promise%5D I will keep looking if i find out what exactly is happening. Edit: It seems to be the If i debug, the type passed into the React render method is an unresolved Promise. |
Good catch, fixed.
I will take a look at that.
I pushed a potential fix in this new dev version: |
@christian-bromann Unfortunately it still occurs. Here are more details: npx next dev
▲ Next.js 14.2.4
- Local: http://localhost:3000
✓ Starting...
✓ Ready in 1001ms
⚠ Your project has `@next/font` installed as a dependency, please use the built-in `next/font` instead. The `@next/font` package will be removed in Next.js 14. You can migrate by running `npx @next/codemod@latest built-in-next-font .`. Read more: https://nextjs.org/docs/messages/built-in-next-font
○ Compiling / ...
✓ Compiled / in 2.2s (644 modules)
GET /_next/static/webpack/db2d32ca0963fe62.webpack.hot-update.json 404 in 2331ms
⚠ Fast Refresh had to perform a full reload. Read more: https://nextjs.org/docs/messages/fast-refresh-reload
Promise { <pending> }
Error: Objects are not valid as a React child (found: [object Promise]). If you meant to render a collection of children, use an array instead.
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6181:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderIndeterminateComponent (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5785:7)
at renderElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5946:7)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6104:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderNode (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6261:12)
at renderHostElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5642:3)
at renderElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5952:5)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6104:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderNode (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6261:12)
at renderChildrenArray (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6213:7)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6141:7)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
Promise { <pending> }
Error: Objects are not valid as a React child (found: [object Promise]). If you meant to render a collection of children, use an array instead.
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6181:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderIndeterminateComponent (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5785:7)
at renderElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5946:7)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6104:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderNode (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6261:12)
at renderHostElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5642:3)
at renderElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5952:5)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6104:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderNode (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6261:12)
at renderChildrenArray (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6213:7)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6141:7)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
Promise { <pending> }
Error: Objects are not valid as a React child (found: [object Promise]). If you meant to render a collection of children, use an array instead.
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6181:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderIndeterminateComponent (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5785:7)
at renderElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5946:7)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6104:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderNode (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6261:12)
at renderHostElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5642:3)
at renderElement (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:5952:5)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6104:11)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
at renderNode (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6261:12)
at renderChildrenArray (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6213:7)
at renderNodeDestructiveImpl (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6141:7)
at renderNodeDestructive (C:\dev\Liebherr.PatternLibrary\examples\nextjs-example\node_modules\react-dom\cjs\react-dom-server.browser.development.js:6076:14)
:14)
{
'$$typeof': Symbol(react.element),
type: 'patternlib-button',
key: null,
ref: null,
props: {
label: 'Hello',
... Added some logging at the bottom of
{
'$$typeof': Symbol(react.element),
type: 'patternlib-button',
key: null,
ref: null,
props: {
label: 'Hello',
... The node only resolves because of my This is what the node looks like to React {
'$$typeof': Symbol(react.element),
type: [AsyncFunction (anonymous)],
key: null,
ref: null,
props: { label: 'Hello' },
_owner: null,
_store: {}
} React components can't be async, can they? Here is what the {
'$$typeof': Symbol(react.element),
type: [Function: Home],
key: null,
ref: null,
props: {},
_owner: null,
_store: {}
} The demo page was reduced to export default function Home(): JSX.Element {
return (
<main>
<PatternlibButton label="Hello" />
</main>
);
} Edit:I get the same error if i use a NextJS page/React component and add export default async function Home(): Promise<JSX.Element> {
return (
<main>
Test
</main>
);
} The problem is here:
|
I dont really understand whats happening here. As a Next developer just wanting to use Ionic components in my app, my (possibly oversimplistic?) expectation is to know what Ionic components I can render on the server and which are client only. There seems to be a lot more going on here at a lower level of abstraction. Is the outcome of this being able to render (some?) Ionic components on the server? |
@mayerraphael did you check with the latest dev version?
Yes. |
So... a finite (documented?) set of layout only Ionic server (isomorphic?) components? |
Well there are many other projects that use this React Output Target for component libraries that have interest in this, e.g. see some of the reference links in ionic-team/stencil#5831. Even if a component is for layout only, the DX is much better if it can also be used on the server side. |
It seems to only work with the new AppRouter of NextJS, not with the PageRouter. As everything is async i guess there is no chance for PageRouter support. The PageRouter is not recommended anymore with Next 14+, but it is also not deprecated, see https://nextjs.org/docs/pages Please put that it is only compatible with the AppRouter into your documentation or i guess you might get some bug tickets :) I've updated my repository with two NextJS examples, one using PageRouter and one with the AppRouter. There you can replicate the PageRouter problem. https://github.com/mayerraphael/stencil-dsd-ssr-playground Thanks for your good work @christian-bromann |
@christian-bromann Summary of our current issues: GlobalScripts execution missingNow the React components internally call It would be best the user could disable the internal
Breaking changeIt's not possible to run the new React Output Target with the old Just updating to the new React Output Target would break our existing project (NextJS PageRouter with custom server.js calling hydrate.renderToString()). I guess this change here should be backwards compatible:
This is definitly a step in the right direction, but currently i think it would break many existing React projects. The new concept should either be a Output Target VersionIs there a reason why the latest stable react-output-target is 0.5.3 from a year ago and newer dev version are available (like the one we test now) but never made it to stable? The interface of reactOutputTarget options has a breaking change (proxyfiles removed, componentCorePackage removed). |
First off, thanks a lot @mayerraphael for all the feedback!
Yes, for now we will only support AppRouter as Stencils serialization function is async and I assume it would require larger efforts to support a sync version of that.
I am not sure if global scripts should be part of the hydration process. It is hard to say as I don't have too much knowledge about what people do in the globalscripts but I believe it is mostly setting up the browser environment which is not needed on the server side. For example there is no need to call I personally would not recommend the use of global scripts at all as they can potentially block the app from being rendered and slow down your app load performance.
This way this is meant to be used is: the React output target creates a
I am not sure how you've used the React Output Target before but I believe it always has been this way that it would generate wrapper files. The current stable version of the React output target doesn't use
You still can do that. In your
Note that this new feature is mainly to be used for meta frameworks such as Next or Remix (haven't tested on that one yet). You can still run your own hydration process using
Not sure if I understand this issue. Do you have a concrete use case?
The SSR support for the React Output Target is behind a flag (
There hasn't been any updates for quite some time. Just recently we have updated the React Output Target which we are still testing internally including this new feature. |
df8c997
to
1d2791a
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.
Great work! I don't see anything that is blocking.
I have one question on the compatibility with environments.
}: { | ||
stencilPackageName: string; | ||
components: ComponentCompilerMeta[]; | ||
customElementsDir: string; | ||
outDir: string; | ||
esModules?: boolean; | ||
experimentalUseClient?: boolean; |
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.
Will older environments be able to parse (or skip) the use client;
directive annotation in JS files?
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.
Can you specify what you mean by that? Afaik use client;
is a Next.js primitive and can be omitted in all other environments. This patch only emits use client
when components are generated with a hydrate module, suggesting that the user wants enable SSR within Next.js applications. We have to evaluate support for other React frameworks, as this is currently only tested with Next.js.
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.
Ahh I see, the use client;
was moved to only be generated as part of the Next.js component wrappers. The config option was replaced with the hydrate module.
This makes sense - I don't have concerns 👍
|
||
| `customElementsDir` | The directory where the custom elements are saved. This value is automatically detected from the Stencil configuration file for the `dist-custom-elements` output target. If you are working in an environment that uses absolute paths, consider setting this value manually. | | ||
|
||
| `hydrateModule` | For server side rendering support, provide the package for importing the [Stencil Hydrate module](https://stenciljs.com/docs/hydrate-app#hydrate-app), e.g. `my-package/hydrate`. This will generate two files: a `component.server.ts` which defines all component wrappers and a `components.ts` that re-exports these components for importing in your application. | |
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 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.
@smorrisods thanks for pointing this out, mind raising a PR?
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.
I'd be happy to!
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.
Upon further investigation it looks like it has already been fixed in commit 75c6cce! No further PRs needed 🙂.
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?
The current React output target can only be used on the client side. There is no support for server side rendered content, e.g. within a Next.js environment
What is the new behavior?
This patch enhances the React output target to support server side rendering.
Does this introduce a breaking change?
Other information
This is work in progress.