[ssr] Investigate interactions with minified or unminified lit-html

[AndrewJakubowicz]

Which package(s) are affected?

SSR (@lit-labs/ssr)

Description

Issue was raised by Justin here: #4515 (comment)

Because ssr depends on its own lit-html, there is the possibility that the version of lit-html that ssr imports is minified, while the templates/directives being SSR'd are not minified, or vice-versa. E.g. if a library being SSR'd has its own bundled Lit.

Reproduction

No repro, the objective of this issue is to potentially write a repro PR, or add testing.

[keisha-rw]

Hello @AndrewJakubowicz! I hit this issue when I'm trying to use the app router in a Next 14 app. I have a library where we've enabled SSR, and thus far it's been working well when utilizing the components in a Next app with pages router. But as soon as I start using the app router and try to view the page source, I get this issue:

I've attached a simplified reproduction of the issue in the form of two repositories. The design-system-ssr repo is our design system library that exports React-wrapped Lit components with SSR enabled. The next-app-router repo is mostly the bare-bones create-next-app code with the app router enabled. I've added a few npm scripts and updated it to use our library, but that's about it. If you look at the README of the design-system-ssr repo, I've given step-by-step instructions on how to reproduce what I'm seeing. It should be as simple as downloading/unzipping the files and running a few commands.

Is there any work being done on this issue right now? Our organization is going to be enabling the app router for many of our applications soon, and we'd like to continue to have SSR support for those who want to use Next's app router.

Please let me know if you need any more info or if I can help keep this issue moving. I truly appreciate all the Lit team is doing to enable SSR!

design-system-ssr.zip
next-app-router.zip

cc @augustjk

[augustjk]

@keisha-rw this issue might be related #3845 (comment)
I'm not sure why nextjs sometimes fails to use the module with the correct condition but could answer why you're seeing mix of prod and dev versions of lit. For some, forcing it with node options seems to fix it.

Also the lit nextjs plugin currently won't deeply SSR custom elements with the app router as we're not targeting for those files when injecting our patches. I've been meaning to address this but it'll require some clear documentation on limitations. It's looking like deeply server rendering custom elements producing declarative shadow DOM won't work when used within React Server Components. See #3657

[keisha-rw]

Hey @augustjk! Thanks for taking a look! I already had the fix to force dev via node options (I am using it with our pages router implementation), and that didn't seem to do the trick in this instance. I started looking at the different Lit libraries that my next app was pulling in, and I did have @lit-labs/nextjs as a plugin wrapping my next config. I had to force install that using --legacy-peer-deps because this test app that I'm using is on Next 14, so that could definitely be a part of what I was seeing:

What's interesting is that if I removed the @lit-labs/nextjs plugin, and then installed @lit-labs/ssr-react directly in the next app (since I'm using @lit-labs/ssr-react/enable-lit-ssr.js), then the app router works in that it renders my component (in a client component) and I can see the DSD when I go to view the source of the page. Can you see any potential issues with that approach? I can figure out the dependencies in my library to make sure consumers don't have to import @lit-labs/ssr-react manually, but I'm not sure if it will be a problem to remove the @lit-labs/nextjs plugin.

FWIW, I'm ok with using client components for a bit (since they still do render on the server) until you guys can get the declarative shadow dom deeply rendering in RSCs. I'd just like my consumers to be able to pick if they want to use the pages or the app router (or both). Fully supporting RSCs can come later for us!

[augustjk]

Using @lit-labs/ssr-react directly will work for situations where React.createElement() is used to create custom elements. You'd also need to set the jsxImportSource to @lit-labs/ssr-react to handle runtime jsx, but I think that's only for your source code. If you have a dependency in node_modules that is a react component using a custom element inside built with runtime jsx, it won't get SSRed. (This situation is likely very rare)

I'm going to try and work on plugin support for app router and v14 into the nextjs plugin soon.

[keisha-rw]

You are truly a gem, @augustjk! 🌟 Thank you for that. My library sets the jsxImportSource to @lit-labs/ssr-react so I think we're good from that standpoint. I do agree that we're not likely to run into a situation where there's a dependency in node_modules that is a react component using a custom element inside built with runtime jsx, so I'm good with that. I'll reach out again if I run into any further issues, but I really appreciate your timely responses to my questions!

[keisha-rw]

Hey @augustjk! 👋 I've been doing more investigation on this lately and it seems to be working at times, but occasionally I still get this error when using the app router, even with a manual install of @lit-labs/ssr-react and having cross-env NODE_OPTIONS=--conditions=development at the beginning of my start script. I delete the .next folder and re-install our library as a dependency in the Next 14 app each time I'm running the app, so I think there's something with how the modules get installed that might be affecting this, but I haven't determined exactly what that may be yet.

That aside, do you think your monkey-patching updates in this PR #4575 will fix this issue? If so, do you have any idea on when that PR might merge and be released? We'd like to start rolling out SSR with our components in applications that use the app router.

As always, thanks for your work on this!

[augustjk]

I don't think #4575 would fix this particular problem as it doesn't change anything about how user code is imported.

When you say occasionally, is it the same app that sometimes works and sometimes doesn't? Or have you been able to isolate something that consistently causes it like inclusion of particular components? Does the error only happen when you run it in dev mode? If forcing condition doesn't fix, another source of the discrepancy I can think of is there may be pre-bundled code that includes production version of Lit.

I think ideally we'd want to just be able to avoid this whole thing by making things work across both dev and prod builds, but I don't know if that's really feasible without a hard-code of internal minified method names which seems bad for maintainability.

[keisha-rw]

Sorry, I should have clarified that! When I say "occasionally," I mean that it doesn't show when I start the NSK app sometimes, but if I delete the node_modules folder and package-lock.json file and install my dependencies again and restart my Next app (locally), then it will appear. The page and components I'm loading are the same, and I'm not changing how my component library is built. My lib externalizes all lit*|@lit* dependencies in our vite config (build/rollupOptions), by doing this:

rollupOptions: {
      // Externalize deps that shouldn't be bundled
      external: (id: string) => {
        if (id.match(/^@?lit.*$|^react$/g)) {
          return true;
        }

        return false;
      },
      output: {
        entryFileNames: '[format]/[name].js',
        chunkFileNames: '[format]/[name].[hash].js',
      },
    },

(we have multiple vite.config files, but I verified that we're doing this in all our packages)

And I can see in the es build of my library that the lit dependencies are externalized:

cjs also looks good to me:

I'm continuing to investigate and I'll update you as I find more, but thus far I can't see a consistent cause for this.

[keisha-rw]

So now it seems I'm getting this message rather consistently. I'll keep poking around to try to get a better idea of when/why this is popping up again, but I still can't quite get a handle on what's causing it to show up.

Given that the message is coming from ../../node_modules/lit-html/node/development/private-ssr-support.js, I'm assuming the --conditions=development flag is working properly and that there is a production version of lit hanging on somewhere, but I've yet to find it. I threw a bunch of console logs in all the lit dependencies of our library (both the prod and dev versions) to see if maybe one of those was getting included in the build, but I don't see those logs appearing in my Nextjs app when I pull in our built library so it seems like that our library is at least not bundling the lit dependencies with it.

I also noticed that I'm getting Multiple versions of Lit loaded. Loading multiple versions is not recommended. See https://lit.dev/msg/multiple-versions for more information. in my node console output, but npm ls is showing all lit dependencies at the same version (with dedupes looking normal). I am not, however, seeing this warning in the browser console. And there if I log window.litElementVersions I get ['4.0.4'], window.reactiveElementVersions gives ['2.0.4'], and window.litHtmlVersions gives ['3.1.2']. This could be totally unrelated and I may be chasing my tail here, but I'm assuming that if a prod and dev version of lit were being used I would see this message as well.

Please let me know if you have any ideas for what I can try to debug to get to the bottom of this!

[augustjk]

Thanks for the additional info! The deletion of node_modules and package-lock.json was making me think it might be multiple copies of Lit within node_modules, but if npm ls shows everything properly deduped perhaps that's not the root cause.

Multiple versions of Lit loaded might only be happening server-side due to how Next builds and loads code per route, or however it does partial builds for hot module reloading. Perhaps it's something there that's tripping the error. Just looking at the code though, I don't think multiple copies should be tripping it. I wish there could be a minimal repro I could play with but unfortunately I'm not able to spend too much time on this. :(

To be clear, this does not happen if you do a production build?

[keisha-rw]

I think you're onto something with the idea of looking into how Next builds and loads code per route, because when I use the same code for our lib in a Next 13 app with pages router, I don't see this error at all. The rest of my team has been testing our SSR implementation of our components library in the Next 13/pages router as well and no one has hit this issue there. So I'm pretty confident it's not that we actually have a bundled production version of Lit within our code or I'd think we'd be seeing this issue in our pages router app too.

I did create a minimal repro in #4527 (comment) if that helps!

I haven't put this through its paces beyond local development yet, but I can see what happens when I run a production build. I'll try to fake that out locally because we don't have a proper workflow set up with the Next 14/app router app at the moment.

I'll keep you posted as I continue working on this, but in the meantime, I appreciate any insights you may have! Thanks a ton.

[keisha-rw]

I got back to this today and tried to run a production build, but given that this is Next 14, I hit the issue mentioned here: #3657 (comment) and it won't build. So unfortunately I'll need the fix for Next 14 to merge before I can test this out with a production build.

[augustjk]

@keisha-rw sorry for the delay. The Next 14 support and fix to those errors have now been released. I don't think it would do anything for the dev/prod mixup, but you should be able to test it.

[keisha-rw]

@augustjk Thank you!! 🎉 I'll take a look at this hopefully in the next week or two and see how it's working for us. In the meantime, I created a client-side only bundle of our library for Next 14/app router users, so I'm excited to see if we can enable SSR functionality for them! I'll keep you posted.