lit: [ssr] Lit SSR breaks Sass with polyfill
Description
The required Lit SSR global window polyfill breaks Sass, and potentially other tools. cc @jaydanurwin who ran into this issue while trying to record a video on using Lit + Sass in Astro.
Steps to Reproduce
- Write this code in Node.js
await import('@lit-labs/ssr/lib/install-global-dom-shim.js');
// then, Sass fails to load
await import('sass');
- See error:
"Cannot assign to read only property 'fs' of object '#<Object>'
Relevant: https://github.com/mbullington/node_preamble.dart/issues/26
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 18 (8 by maintainers)
@lit-labs/ssrv3.0.0 was just published, which has the following changes that should resolve this issue:window.So, if you’re using
import {render} from '@lit-labs/ssr/lib/render-with-global-dom-shim.js', you should now instead useimport {render} from '@lit-labs/ssr'– and you can remove any imports for@lit-labs/ssr/dom-shim.jsand@lit-labs/ssr/install-global-dom-shim.jsNote you will need to be on at least
v2.6.0oflitfor the automatic shimming to occur.Here are a few things we found after digging around.
Part of the problem is how the
sassnpm package, which is compiled to js from dart, tries to figure out its running environment. As mentioned in the issue linked by jonathantneal mentioned above, https://github.com/mbullington/node_preamble.dart/blob/master/lib/preamble.js#L41-L53 checks forwindowto see if it’s running in Node to do its own polyfilling. The more problematic portion is that the compilation from dart to js also adds this check for the presence ofdocumentin global https://github.com/dart-lang/sdk/blob/main/pkg/compiler/lib/src/js_emitter/main_call_stub_generator.dart#L70-L73 which breaks sass when run after the DOM polyfill.Another issue seems to be arising due to this line https://github.com/withastro/astro/blob/main/packages/renderers/renderer-lit/server-shim.js#L4. I’m not sure on what the purpose of this is is but along with this Window proxy https://github.com/lit/lit/blob/main/packages/labs/ssr/src/lib/dom-shim.ts#L190-L201 it makes it so that
globalisn’t exactly the full nodeglobaldown the line. I think we can investigate looking into removing the Window proxy from our dom shim but perhaps the line from astro lit renderer’s server-shim could also be removed.A couple potential solutions:
Firstly, loading the sass module before any of the DOM shimming by adding
import 'sass';to the top of the@astrojs/renderer-lit/server-shim.jscompletely solves this issue. I don’t know if it would be possible for Astro to let Vite handle the styling preprocessors do their thing first before loading up the shims and doing the rendering, but that should allow sass to not break.Otherwise, we’d need the dart2js compiler to do something besides just checking for absence
documentin global (perhaps check for the presence ofprocess?) to tell how the script should be executed, along with fixing the Window proxy/global issue above. Not polyfillingwindowalone will not fix this sincedocumentwill still be on there.I’m hopeful that Workers… work. It’d be nice to have an isolation and module cache-busting solution that work without flags.
Unfortunately this is not possible because dynamic
import()in any isolated vm context always requires the--experimental-vm-modulesflag. From the docs:@FredKSchott Speaking of that, have you looked into using the isolated vm context approach for Lit SSR in Astro? It does require the
--experimental-vm-modulesflag, and I suppose you could potentially include it in yourastrobinary heading (e.g.#!/usr/bin/env node --experimental-vm-modules) so that users don’t actually need to know about it. This approach should provide full isolation from any interactions with other libraries.If that’s not an option though, we’re also going to start looking into using Node worker threads to provide isolation, which doesn’t require any experimental flags. (@thescientist13 I noticed you are looking into that too! Curious if you have any findings so far.).
Another reason that Astro and other tools may want an isolated rendering approach is for watch modes, which often work with a loop all within the same process. There is no way to bust the ESM cache in Node, so updating a component definition won’t have any effect in that case until the process is restarted. @FredKSchott have you run into this problem with watch mode using the global style Lit SSR technique yet?
Even simpler, adding
as suggested here makes the sass script run.
@matthewp Thanks for the reply. Totally agree that the
import 'sass'doesn’t belong there, that was purely testing on my part to see if changing the import order could make it work. Since we can’t reorder the loading, we’ll have to makesasswork even in a polyfilled environment.I’ve filed an issue with the Dart team regarding script execution in Node with document polyfilled. In the meantime, we can sort of hack around the existing dart2js generated code by adding something like this to the shim (credit to @aomarks for the idea):
This allows the script to run when we get here: https://github.com/dart-lang/sdk/blob/ef66f47631b31ad149d425a80dd1295020711173/pkg/compiler/lib/src/js_emitter/main_call_stub_generator.dart#L88-L90
This shouldn’t be an issue if we load all the code into a fresh VM context, like we do with the JS module loader. Since we want a version that can work without the
--experimental-vm-modulesflag, maybe there’s a way to use plain dynamicimport()in a classic script vm context so that we get a fresh global, just without the module loading hooks.