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

Update lit version to v2 #1445

Merged
merged 3 commits into from
Sep 28, 2021
Merged

Update lit version to v2 #1445

merged 3 commits into from
Sep 28, 2021

Conversation

stramel
Copy link
Contributor

@stramel stramel commented Sep 27, 2021

Changes

Bumps to the official v2 release of Lit and the official v1 release of @lit-labs/ssr in the lit renderer

Testing

Tests are passing

Docs

Shouldn't need any documentation update.

@stramel stramel requested a review from a team as a code owner September 27, 2021 19:49
@changeset-bot
Copy link

changeset-bot bot commented Sep 27, 2021

🦋 Changeset detected

Latest commit: e758769

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/renderer-lit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Sep 27, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

astro-docs – ./docs

🔍 Inspect: https://vercel.com/pikapkg/astro-docs/BDKuN1KSzVWMkKRP13xasYPicaoD
✅ Preview: Canceled

[Deployment for e758769 canceled]

astro-www – ./www

🔍 Inspect: https://vercel.com/pikapkg/astro-www/7QAMXLKr9A5ypnRB57mDihETm6jy
✅ Preview: Canceled

[Deployment for e758769 canceled]

@vercel vercel bot temporarily deployed to Preview – astro-docs September 27, 2021 19:49 Inactive
@vercel vercel bot temporarily deployed to Preview – astro-www September 27, 2021 19:49 Inactive
@vercel vercel bot temporarily deployed to Preview – astro-docs September 27, 2021 19:50 Inactive
@vercel vercel bot temporarily deployed to Preview – astro-www September 27, 2021 19:50 Inactive
@matthewp
Copy link
Contributor

Thanks! It looks like it's having trouble accessing document: https://github.com/snowpackjs/astro/pull/1445/checks?check_run_id=3724759035#step:9:404

The Lit renderer installs Lit's window shim on the global so it should be there: https://github.com/snowpackjs/astro/blob/main/packages/renderers/renderer-lit/server-shim.js#L1

Might need to debug why we are getting this error.

@stramel
Copy link
Contributor Author

stramel commented Sep 27, 2021

@matthewp It seems strange that it passed on Node v12 to me but not Node v14.

I will keep trying to dig into it

@stramel
Copy link
Contributor Author

stramel commented Sep 27, 2021

@matthewp Seems all the errors stem from an update to @lit-labs/ssr. Everything passed with Lit v2 update by itself

@stramel
Copy link
Contributor Author

stramel commented Sep 27, 2021

@matthewp Sorry for all the pings. I found the root cause but I'm not sure what solution we want to go with.

Root issue: @lit-labs/ssr no longer shims window.global to window. lit/lit@2043eb0

So by adding window.global = window after the shim is installed, everything works. This feels very hacky to me.

I'm also quite confused why all React, Vue, Preact, and Markdown are all reliant upon this shim being installed in a test. Seems to me that this is just bad setup for those tests. If they were to run before that shim were to be installed, they would all fail as they are currently.

@matthewp
Copy link
Contributor

Thanks for the research @stramel!

I don't think that those other tests need the shim, but rather that the existence of the shim causes their code to branch (due to some feature detection I imagine). So this is kind of a good thing, because it's catching "what happens when you have Lit & React/etc. in the same project).

I'm fine with adding the window.global if that's all it takes.

@vercel vercel bot temporarily deployed to Preview – astro-docs September 28, 2021 17:32 Inactive
@vercel vercel bot temporarily deployed to Preview – astro-www September 28, 2021 17:32 Inactive
@matthewp matthewp merged commit 806dcd8 into withastro:main Sep 28, 2021
@stramel stramel deleted the ms/bump-lit branch September 28, 2021 19:41
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.

2 participants