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

Implement the Astro.request RFC #2913

Merged
merged 7 commits into from
Mar 29, 2022
Merged

Implement the Astro.request RFC #2913

merged 7 commits into from
Mar 29, 2022

Conversation

matthewp
Copy link
Contributor

Changes

Testing

Tests updated

Docs

@changeset-bot
Copy link

changeset-bot bot commented Mar 28, 2022

🦋 Changeset detected

Latest commit: 093bf9a

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

This PR includes changesets to release 1 package
Name Type
astro Minor

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) pkg: example Related to an example package (scope) labels Mar 28, 2022
Copy link
Contributor

@bholmesdev bholmesdev left a comment

Choose a reason for hiding this comment

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

Left one requestion about logging, but Let's GET this money 💰

/** set props for this astro component (along with default values) */
props: Record<string, number | string | any>;
/** get information about this page */
request: AstroRequest;
request: Request;
Copy link
Contributor

Choose a reason for hiding this comment

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

love this change 👏

Copy link
Member

Choose a reason for hiding this comment

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

seriously, it's the little things :)

packages/astro/src/core/request.ts Outdated Show resolved Hide resolved
@matthewp matthewp force-pushed the astro-request-rfc branch from a574111 to e16fbab Compare March 28, 2022 15:24
@github-actions github-actions bot added the test label Mar 28, 2022
}

// Headers are only available when using SSR.
const request = createRequest({
Copy link
Member

@FredKSchott FredKSchott Mar 28, 2022

Choose a reason for hiding this comment

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

TIL that Node.js http.IncomingMessage wasn't a Request object... For some reason I thought that that had always been the case!

@FredKSchott
Copy link
Member

Did my best to fix merge conflicts for you. If you need to, you can force-push your local branch back up github to revert my fixes.

@matthewp matthewp merged commit ecc6a48 into main Mar 29, 2022
@matthewp matthewp deleted the astro-request-rfc branch March 29, 2022 11:35
@github-actions github-actions bot mentioned this pull request Mar 29, 2022
SiriousHunter pushed a commit to SiriousHunter/astro that referenced this pull request Feb 3, 2023
* Implement the Astro.request RFC

* Disable console warnings eslint

* Use our logger

* Adds a changeset

* Fix tests that depend on params, canonicalURL, URL

Co-authored-by: Fred K. Schott <[email protected]>
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) pkg: example Related to an example package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants