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

Finish fix for #416 #452

Closed
wants to merge 1 commit into from
Closed

Conversation

gilbert
Copy link
Contributor

@gilbert gilbert commented May 3, 2024

See here for details: #416 (comment). In a nutshell, you only need globalThis.

@CLAassistant
Copy link

CLAassistant commented May 3, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@Ansonhkg Ansonhkg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for the PR. I'm not sure if this will completely fix the issue, because we are now using cross-fetch PR, which uses global under the hood. Therefore, the same problem might occur.

image

May I suggest to configure your build tool to shim the global object while we investigate this issue?

eg. in vite

import { defineConfig } from "vite";
import react from "@vitejs/plugin-react-swc";


export default defineConfig({
  plugins: [react()],
  define: {
    global: "window",
  },
});

cc @MaximusHaximus @joshLong145 can you take a look?

Ansonhkg added a commit that referenced this pull request May 4, 2024
@gilbert
Copy link
Contributor Author

gilbert commented May 15, 2024

Indeed, I'm upgrading to lit v6 and am now running into the cross-fetch issue.

Either way, there's no reason to use global over globalThis, and I see a4dd28f implemented this fix. Thanks!

@gilbert gilbert closed this May 15, 2024
@Ansonhkg
Copy link
Collaborator

Ansonhkg commented May 15, 2024

cross-fetch

Thanks, we are considering ripping out cross-fetch completely, as it's too hard to satisfy all environments, we will just let users to polyfill themselves and provide configuration guides in the docs instead.

I could however publish a temp. version for you without cross-fetch if that would help?

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

Successfully merging this pull request may close these issues.

3 participants