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

  1. 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');
  1. 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)

Most upvoted comments

@lit-labs/ssr v3.0.0 was just published, which has the following changes that should resolve this issue:

  • The global DOM shim no longer defines a global window.
  • Even further, the global DOM shim is no longer required at all, and it is strongly preferred that you remove it. Lit will now automatically import and use shims for the APIs it needs when running in Node.

So, if you’re using import {render} from '@lit-labs/ssr/lib/render-with-global-dom-shim.js', you should now instead use import {render} from '@lit-labs/ssr' – and you can remove any imports for @lit-labs/ssr/dom-shim.js and @lit-labs/ssr/install-global-dom-shim.js

Note you will need to be on at least v2.6.0 of lit for the automatic shimming to occur.

Here are a few things we found after digging around.

Part of the problem is how the sass npm 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 for window to 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 of document in 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 global isn’t exactly the full node global down 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.js completely 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 document in global (perhaps check for the presence of process?) to tell how the script should be executed, along with fixing the Window proxy/global issue above. Not polyfilling window alone will not fix this since document will 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.

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-modules flag, maybe there’s a way to use plain dynamic import() in a classic script vm context so that we get a fresh global, just without the module loading hooks.

Unfortunately this is not possible because dynamic import() in any isolated vm context always requires the --experimental-vm-modules flag. From the docs:

importModuleDynamically <Function> Called during evaluation of this module when import()
is called. If this option is not specified, calls to import() will reject with
`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`. This option is part of the experimental modules API.
We do not recommend using it in a production environment.

@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-modules flag, and I suppose you could potentially include it in your astro binary 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

document.currentScript = null;

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 make sass work 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):

document.scripts = [{
    addEventListener(_event, cb) {
        cb({ target: null });
    },
    removeEventListener() {}
}];

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-modules flag, maybe there’s a way to use plain dynamic import() in a classic script vm context so that we get a fresh global, just without the module loading hooks.