-
-
Notifications
You must be signed in to change notification settings - Fork 38
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(netlify): Netlify Adapter v4 #84
Conversation
Co-authored-by: Matt Kane <m@mk.gg>
Co-authored-by: Matt Kane <m@mk.gg>
Co-authored-by: Jacklyn <70537879+jacklyn-net@users.noreply.github.com>
That would be nice, I guess! We already mock the context in |
You'll have to add a vite dev server (connect server) middleware, and set |
Wow, that's pretty cool! I'll definitely implement that. |
Besides that, are there any other changes you'd like me to make @lilnasy @alexanderniebuhr @ematipico? From what i'm seeing, all the big ones are addressed and the only missing thing is the docs review. What else do we need to work towards merging? |
I will be testing out the functionality later today. As for the big picture, I would be more conformable if the default story was "node only" - for both SSR and middleware, with edge the opt-in. |
Just checking in, I don't have anything to add for now. But I'll make sure to get the docs review this week for you! :) |
Inspired from withastro@53775ba Co-Authored-By: Arsh <cherrycoloredfunk@icloud.com>
I've implemented the mocked
Cool, will do! I thought the edge was already opt-in, and docs + comments said that - but I noticed the actual code was making it opt-out. Fixed that in 0ac9466. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whew! A major release is exciting! 🥳
Left some suggestions for docs here! (Note: since there was a comment, yes, informing readers of CHANGES goes in the changeset... and there were a lot, so this is the place for them! The README is for "how things just are" and is NOT the place to discuss changes from past versions. So the content there should just be about how it works now.)
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@sarah11918 wow, what a review! I always love docs review because the prose reads so much better after that :D I've made most of the changes you requested, and left comments to the two remaining ones. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing all my comments!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unblocking, since we have docs review
Awesome, thanks for approving! I think now we're only waiting for the docs copy to be finalised. |
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@Skn0tt Just checking in, if you are ready, I'll do another quick sanity check later tonight and get this merged 🚀 |
Sounds great! Maybe we can merge today + release early monday, so we can respond better to questions in Discord. |
@Skn0tt No problem, I'll coordinate everything, so we can get the release on Monday morning CET timezone! :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did another run through the changes, and they lgtm.
Thank you for the PR! @Skn0tt
I'll get this merged now and released on Monday morning! 🚀
Awesome, thanks for all the reviews! I'll keep an eye on Discord and GitHub for any issues that pop up. If I miss any, feel free to tag me in related issues - happy to help. |
Changes
This PR overhauls the Netlify adapter, aiming to make best use of Netlifys platform & streamlining config.
Read the changeset for a detailed overview, but here's the main changes:
netlify-edge-middleware.ts
fileTesting
Tests were brought up-to-date.
Docs
README was updated.