Skip to content
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

🐛 BUG: Window is not defined after using lit integration when deployed on netlify #3126

Closed
1 task
thepassle opened this issue Apr 17, 2022 · 9 comments · Fixed by #3158 or #3164
Closed
1 task

Comments

@thepassle
Copy link
Contributor

What version of astro are you using?

1.0.0-beta.8

Are you using an SSR adapter? If so, which one?

netlify

What package manager are you using?

npm

What operating system are you using?

mac

Describe the Bug

I added the lit integration to my project:

import { defineConfig } from 'astro/config';
+ import lit from '@astrojs/lit';
import netlify from '@astrojs/netlify';
import { vitePluginCommonjs } from 'vite-plugin-commonjs'

export default defineConfig({
  site: 'https://my-astro-course.netlify.app',
  adapter: netlify(),
+ integrations: [lit()],
  vite: {
    plugins: [
      vitePluginCommonjs()
    ]
  },
});

Which locally worked fine, but when deployed to netlify, gives me the following error:

Screenshot 2022-04-17 at 09 37 13

Link to Minimal Reproducible Example

astro.new

Participation

  • I am willing to submit a pull request for this issue.
@thepassle
Copy link
Contributor Author

Looks like the @astrojs/netlify adapter indeed excludes window and document from being polyfilled, but those are probably needed to SSR custom elements:

https://github.com/withastro/astro/blob/main/packages/integrations/netlify/src/netlify-functions.ts#L6-L8

polyfill(globalThis, {
  exclude: 'window document',
});

Is there a particular reason window and document are excluded?

@matthewp
Copy link
Contributor

The goal of that polyfill code is not to provide a full browser environment. It's really just to provide a few things that are present in some server-runtimes but not others, like fetch and friends (Request, Response, Headers, etc). The lit integration does attempt to provide a minimal browser environment and should be polyfilling those itself.

@thepassle
Copy link
Contributor Author

I think the lit integration does put a window on the globalThis though. Im also unsure why this works locally, but not after deployment

@thepassle
Copy link
Contributor Author

Hrm thats weird, i was definitely getting that error after adding the lit integration. Ill try to update my dependencies and redeploy tomorrow to see if the problem is still there

@thepassle
Copy link
Contributor Author

thepassle commented Apr 21, 2022

Yeah, this is definitely not working as expected

Reproduction:

  • npm init astro@latest
  • npm i -S lit @webcomponents/template-shadowroot @astrojs/lit @astrojs/netlify
  • Update astro.config.mjs:
import { defineConfig } from 'astro/config';
+ import netlify from '@astrojs/netlify';
+ import lit from '@astrojs/lit';

export default defineConfig({
+  integrations: [lit()],
+  adapter: netlify()
});
  • Add a simple lit component to src/pages/my-el.js:
import { LitElement, html } from 'lit';

export const tagName = 'my-element';

class MyElement extends LitElement {
  render() {
    return html` <p>Hello world! From my-element</p> `;
  }
}

customElements.define(tagName, MyElement);
  • Import and use it in index.astro:
---
+ import './my-el.js';
---
<html lang="en">
	<head>
		<meta charset="utf-8" />
		<meta name="viewport" content="width=device-width" />
		<title>Astro</title>
	</head>
	<body>
		<h1>Astro</h1>
+		<my-element></my-element>
	</body>
</html>

image

Locally this does work:
image

If I deploy it without the lit integration, it also works.
image

@thepassle
Copy link
Contributor Author

thepassle commented Apr 21, 2022

Ok so I dug a little bit more and it seems like this is a problem in the build. E.g.: if I use a lit component, it imports litelement, which uses some browser apis (like for example some stuff on the window, document, etc). This gets shimmed by lit-ssr. My astro build output (with the netlify adapter) looks like:

image

However, the window shim only gets installed way later in the output bundle (see line nr):

image

So when litelement gets imported at line 4, it'll try to use browser apis that have not been shimmed yet, because that only happens on line 877

You can reproduce this locally by cloning this repro, running the build, and looking at the netlify/functions/entry.mjs: https://github.com/thepassle/astro-lit-netlify-repro

A fix for this could be to somehow make sure the bundler loads the dom shim module first, but i dont think im knowledgeable enough about astros internals/build process

@hippotastic
Copy link
Contributor

hippotastic commented Apr 21, 2022

Okay, so I've done a little testing myself as well, and the error seems to occur because the netlify/functions/entry.mjs file generated by astro build includes your import line coming from src/pages/my-el.js:

import { LitElement, html } from 'lit';

As imports are processed before executing any other code in the entry.mjs file, the following happens:

  1. LitElement from lit gets imported
  2. As LitElement extends ReactiveElement, @lit/reactive-element gets imported
  3. @lit/reactive-element imports the file ./css-tag.js from its own package
  4. css-tag.js attempts to access window.ShadowRoot, which causes the error you're seeing.

I believe that we could fix this error by exporting server-shim.js from the @astrojs/lit package and adding an import '@astrojs/lit/server-shim.js' statement to netlify/functions/entry.mjs during build BEFORE any of the imports contributed by components.

Doing our shimming in an import statement would ensure that the shim gets executed before code from other imports. The current approach to inject our shim as regular code into the functions entrypoint bundle doesn't work due to the way imports are handled.

CCing @matthewp to get his opinion on that as well.

@thepassle
Copy link
Contributor Author

thepassle commented Apr 21, 2022

I think the larger problem is that imports in your components (e.g. src/components/my-element.js)
get externalized in the final build. In the build output, this then leads to any imports you may use in your components to being placed as external imports at the top of the bundle, when the shim hasnt been applied yet. You can work around it by adding those imports to the vite.ssr.noExternal config, which then causes them to be bundled in the output, but maybe it would make more sense to bundle those imports by default as well?

@matthewp
Copy link
Contributor

matthewp commented Apr 21, 2022

Think I have a fix. Since lit dependencies on the renderer being a super-dependency, it must always be bundled.

I have a breaking test now, the previous test was passing because the window was already set by something else in the test environment. Deleting the window before importing the ssr build causes it to break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants