Skip to content

Conversation

@ematipico
Copy link
Member

@ematipico ematipico commented Nov 19, 2024

Changes

The middleware is now dynamically imported, which means we don't need to have it at the top of entry.mjs

Closes PLT-2659

Testing

I tested locally and checked the emitted file. That import is unused, so there's no need for new tests.

Current CI should pass

Docs

N/A

@changeset-bot
Copy link

changeset-bot bot commented Nov 19, 2024

🦋 Changeset detected

Latest commit: c548eb2

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

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

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Nov 19, 2024
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 19, 2024

CodSpeed Performance Report

Merging #12477 will degrade performances by 11.86%

Comparing fix/middleware-import-ssr (c548eb2) with main (733d6c1)

Summary

❌ 1 (👁 1) regressions
✅ 5 untouched benchmarks

Benchmarks breakdown

Benchmark main fix/middleware-import-ssr Change
👁 Rendering: streaming [true], .mdx file 1 s 1.2 s -11.86%

@florian-lefebvre
Copy link
Member

Any reason why changes about globalThis are not in this PR?

@ematipico
Copy link
Member Author

ematipico commented Nov 19, 2024

Any reason why changes about globalThis are not in this PR?

This fix has to be shipped regardless of astro:env. We generate an incorrect code nonetheless. Myself and Matthew agreed that we can ship this fix while we wait for a good fix for astro env

Copy link
Member

@florian-lefebvre florian-lefebvre left a comment

Choose a reason for hiding this comment

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

This looks good but I think we should also get rid of the generated middleware.mjs file, as I did in #12326

@ematipico
Copy link
Member Author

I'll submit a new PR if that's OK with you @florian-lefebvre

@ematipico ematipico merged commit 46f6b38 into main Nov 19, 2024
15 checks passed
@ematipico ematipico deleted the fix/middleware-import-ssr branch November 19, 2024 16:12
This was referenced Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants