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

refactor(core): remove middleware from actions #13262

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ematipico
Copy link
Member

Changes

This PR a start to make Action more "core aware". In this PR the use of the middleware is completely removed, and its logic is moved inside the render-context.tsrender function, in particular the lastNext, which is the last function that is called during the rendering phase.

Testing

Current tests and functionality should stay the same. The Actions middleware was already set to be the last one, which is the same as we just moved inside the lastNext callback we have in core.

Docs

N/A

Copy link

changeset-bot bot commented Feb 17, 2025

🦋 Changeset detected

Latest commit: 0cf031e

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 Feb 17, 2025
@ematipico ematipico force-pushed the refactor/astro-actions branch from fd88009 to e285d9e Compare February 17, 2025 13:57
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.

Although this is a refactor, maybe should we have a changeset to be safe in case people face issues

@ematipico ematipico force-pushed the refactor/astro-actions branch 2 times, most recently from b9ef104 to bd84a49 Compare February 17, 2025 14:02
Copy link

codspeed-hq bot commented Feb 17, 2025

CodSpeed Performance Report

Merging #13262 will not alter performance

Comparing refactor/astro-actions (0cf031e) with main (c5755e1)

Summary

✅ 6 untouched benchmarks

@ematipico ematipico marked this pull request as draft February 17, 2025 15:44
@ematipico
Copy link
Member Author

Turns out it is not as simple as that, it requires a full refactor of the architecture. I'll see to that

@ematipico ematipico force-pushed the refactor/astro-actions branch from 64e2187 to c2042d2 Compare February 18, 2025 15:19
@ematipico
Copy link
Member Author

I checked the e2e test that failed, and the issue isn't much clear. The db package fails to run the seed file, however it doesn't happen when I run the dev server for the fixture.

Also, when running the fixture locally, the likes action (the action) works as expected, but the query doesn't return the expected data. This doesn't happen for the comments action.

As for now, I commented on them.

@ematipico ematipico force-pushed the refactor/astro-actions branch from f47d495 to 0cf031e Compare February 20, 2025 12:55
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