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

Adapter enhancements #9661

Merged
merged 8 commits into from
Jan 17, 2024
Merged

Adapter enhancements #9661

merged 8 commits into from
Jan 17, 2024

Conversation

ematipico
Copy link
Member

@ematipico ematipico commented Jan 11, 2024

lilnasy and others added 3 commits January 8, 2024 19:27
* feat(app): writeResponse for node-based adapters

* add changeset

* Apply suggestions from code review

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* Apply suggestions from code review

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* add examples for NodeApp static methods

* unexpose createOutgoingHttpHeaders from public api

* move headers test to core

* clientAddress test

* cookies test

* destructure renderOptions right at the start

---------

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
* Fallback node standalone to localhost

* Update .changeset/tame-squids-film.md
Copy link

changeset-bot bot commented Jan 11, 2024

🦋 Changeset detected

Latest commit: 7971af9

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: example Related to an example package (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Jan 11, 2024
@ematipico ematipico added this to the 4.2.0 milestone Jan 11, 2024
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.

Tiny grammar fix on a changeset, but also, can you please check the docs page for the Adapter API and just double check whether anything here would require a change to any code snippets, or any extra documentation here: https://docs.astro.build/en/reference/adapter-reference/

I know this is one of our least supported pages in docs 😅 but if you're changing the adapter API and adding new helper functions, then it might mean that something on this page needs updating, so we should confirm!

.changeset/tame-squids-film.md Outdated Show resolved Hide resolved
lilnasy and others added 2 commits January 12, 2024 22:02
* descriptive names for files and functions

* update tests

* add changeset

* appease linter

* Apply suggestions from code review

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>

* `server-entrypoint.js` -> `server.js`

* prevent crash on stream error (from PR 9533)

* Apply suggestions from code review

Co-authored-by: Luiz Ferraz <luiz@lferraz.com>

* `127.0.0.1` -> `localhost`

* add changeset for fryuni's fix

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

---------

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: Luiz Ferraz <luiz@lferraz.com>
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
* refactor

* add changeset

* bump peer dependencies
@lilnasy lilnasy marked this pull request as ready for review January 13, 2024 01:47
@github-actions github-actions bot added the semver: minor Change triggers a `minor` release label Jan 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 minor 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.

For the sake of completeness, approving the changeset for docs as long as the little tense grammar nit is fixed, one way or the other!

ematipico and others added 2 commits January 15, 2024 16:11
@ematipico ematipico merged commit d6edc75 into main Jan 17, 2024
14 checks passed
@ematipico ematipico deleted the adapter-enhancements branch January 17, 2024 13:10
@astrobot-houston astrobot-houston mentioned this pull request Jan 17, 2024
@porl
Copy link

porl commented Jan 20, 2024

For some reason when I use this merged version 8.0.0 rather than 7.0.4 I get an error on my docker server with Traefik of bad gateway. Rolling back to 7.0.4 works as expected. No errors in any logs I can find and the node server claims it is running fine on the correct IP and port. Running the node server on my local dev machine has no problems, so I assume it is something specific to the way Traefik tries to access the port.

Not sure how I can replicate this outside of my production server I'm afraid, so I don't know how helpful this is.

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) pkg: integration Related to any renderer integration (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port conflict
5 participants