solidjs-community / solid-transition-group

SolidJS components for applying animations when children elements enter or leave the DOM.
https://solid-transition-group.netlify.app
MIT License
254 stars 14 forks source link

`ReferenceError: Element is not defined`: SSR breaks when using `solid-transition-group` on cloudflare workers #33

Open Odas0R opened 1 year ago

Odas0R commented 1 year ago

Here's the reproduction url:

https://github.com/Odas0R/solid-group-transition-bug

Everything you need to do is on the URL to throw the error. I'm using wrangler cli which emulates a worker environment. Code also breaks in production.

This is the code that breaks:

import { createSignal } from "solid-js"
import { Transition } from "solid-transition-group"

export const SolidTransition = () => {
  const [isOpen, setOpen] = createSignal(false)

  return (
    <div>
      <Transition>{isOpen() && <div>hello</div>}</Transition>
    </div>
  )
}

On the astro page:

---
import Layout from '../../layouts/Layout.astro';
import { SolidTransition } from '../../components/SolidTransition.tsx';

export const prerender = false;
---

<Layout title="Welcome to Astro.">
    <main>
        <h1>SSR</h1>
        <SolidTransition client:load />
    </main>
</Layout>

The error:

[mf:wrn] Service bindings are experimental. There may be breaking changes in the future.
[mf:inf] Worker reloaded! (171.47KiB)
[mf:inf] Listening on 0.0.0.0:3000
[mf:inf] - http://127.0.0.1:3000
[mf:inf] - http://192.168.31.106:3000
[mf:inf] - http://172.17.0.1:3000
[mf:inf] Updated `Request.cf` object cache!
GET /preview/lol 404 Not Found (6.99ms)
[mf:err] GET /ssr: ReferenceError: Element is not defined
    at Ui (/tmp/astro-test/wow/dist/_worker.js:60:23785)
    at tn (/tmp/astro-test/wow/dist/_worker.js:60:23813)
    at Object.fn (/tmp/astro-test/wow/dist/_worker.js:60:24004)
    at Bi (/tmp/astro-test/wow/dist/_worker.js:60:19150)
    at Te (/tmp/astro-test/wow/dist/_worker.js:60:18936)
    at Z (/tmp/astro-test/wow/dist/_worker.js:60:17075)
    at qi (/tmp/astro-test/wow/dist/_worker.js:60:23998)
    at Wi (/tmp/astro-test/wow/dist/_worker.js:60:25730)
    at an (/tmp/astro-test/wow/dist/_worker.js:60:26836)
    at ra (/tmp/astro-test/wow/dist/_worker.js:71:164)
GET /ssr 200 OK (173.41ms)
GET /_astro/index.12c5fa68.css 304 Not Modified (5.66ms)
Odas0R commented 1 year ago

Here's a possible fix:

import type { Component, JSX } from "solid-js";
import { isServer } from "solid-js/web";
import type { TransitionProps } from "solid-transition-group";
import { Transition as SolidTransition } from "solid-transition-group";

export const Transition: Component<
  TransitionProps & {
    children: JSX.Element;
  }
> = (props) => {
  if (isServer) {
    return props.children;
  }
  return <SolidTransition {...props} />;
};
thetarnav commented 1 year ago

Interesting I've removed all export conditions from dependencies and decided to just rely on isServer exported by solid, but as it turns out, isServer is different when imported in a module and locally. This means that on the server there are two different versions of solid running which can be very bad.

image

Not sure if that's an issue of @astrojs/solid-js or the wrangler.

Odas0R commented 1 year ago

Huh! wierd :')

So that's why it was throwing an error, and why it works locally on the fix above.

I tried to give it a look but found no answer to why... But it could be any one of the two, since the cloudflare adapter wraps everything on the _worker.js, when acessing SSR Pages, it might conflict there.

Maybe an additional check is needed to see if it is on a Worker environemnt (a.k.a server environment)?

thetarnav commented 1 year ago

Maybe an additional check is needed to see if it is on a Worker environemnt (a.k.a server environment)?

Nobody's gonna do that unfortunately. isServer is the most idiomatic check we have.

Besides it would only hide the issue (like your Transition wrapper does) which is that the bundler doesn't respect export conditions in upstream dependencies. You have a client version of solid running on the server.

This is something worth bringing up with the astro team instead.

Odas0R commented 1 year ago

I'll open an issue there to see if we can arrange a fix. Thanks for debugging this!

thetarnav commented 1 year ago

@Odas0R any updates on this? can you link the issue?

Odas0R commented 1 year ago

I'm so sorry @thetarnav, it has been crazy days since the past month. I did not create the issue and forgot completely to notice you!

I'll probably only be available in the summer :')

If till now no one looked into this, I'll try to debug more.