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

feat(next): TODOs #11987

Merged
merged 11 commits into from
Sep 13, 2024
Merged

feat(next): TODOs #11987

merged 11 commits into from
Sep 13, 2024

Conversation

florian-lefebvre
Copy link
Member

@florian-lefebvre florian-lefebvre commented Sep 13, 2024

Changes

  • Starts handling TODOs
  • Astro.locals can no longer be reassigned.
  • app.render() now takes 2 arguments instead of 3.

Testing

  • They pass

Docs

Copy link

changeset-bot bot commented Sep 13, 2024

🦋 Changeset detected

Latest commit: f79a6a5

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 pkg: astro Related to the core `astro` package (scope) docs pr labels Sep 13, 2024
@@ -228,7 +228,6 @@ export function createGetDataEntryById({

const lazyImport = await getEntryImport(collection, id);

// TODO: AstroError
Copy link
Member Author

Choose a reason for hiding this comment

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

This will be removed in Astro 6.0, not worth doing it now

packages/astro/src/types/public/integrations.ts Outdated Show resolved Hide resolved
@Princesseuh Princesseuh self-assigned this Sep 13, 2024
@Princesseuh Princesseuh marked this pull request as ready for review September 13, 2024 14:54
@@ -12,7 +12,7 @@ export async function emitESMImage(
id: string | undefined,
/** @deprecated */
_watchMode: boolean,
// FIX: in Astro 5, this function should not be passed in dev mode at all.
// FIX: in Astro 6, this function should not be passed in dev mode at all.
Copy link
Member

Choose a reason for hiding this comment

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

we won't get to this right now, it's.. tough

@github-actions github-actions bot added the semver: major Change triggers a `major` release label Sep 13, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a major changeset. A reviewer will merge this at the next release if approved.

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thanks! Left some suggestions/questions!

.changeset/cool-mangos-shop.md Outdated Show resolved Hide resolved
'astro': patch
---

App class now accepts renderOptions
Copy link
Member

@sarah11918 sarah11918 Sep 13, 2024

Choose a reason for hiding this comment

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

Just checking, does this need updating in the docs here: https://docs.astro.build/en/reference/adapter-reference/#apprenderrequest-request-options-renderoptions and we should mention it in the upgrade guide as a Breaking Change

'astro': patch
---

App class now accepts renderOptions
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
App class now accepts renderOptions
Adds a new `renderOptions` object for `App/class`

Not sure about the syntax for App class, but it likely needs something!

Copy link
Contributor

Choose a reason for hiding this comment

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

App is a class, this is for the render method. I'll update to something.

.changeset/blue-sloths-stare.md Outdated Show resolved Hide resolved
.changeset/blue-sloths-stare.md Outdated Show resolved Hide resolved
matthewp and others added 2 commits September 13, 2024 13:53
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a major changeset. A reviewer will merge this at the next release if approved.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a major changeset. A reviewer will merge this at the next release if approved.

@matthewp matthewp merged commit bf90a53 into next Sep 13, 2024
14 checks passed
@matthewp matthewp deleted the feat/next-todos branch September 13, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs pr pkg: astro Related to the core `astro` package (scope) semver: major Change triggers a `major` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants