-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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: add @remix-run/azure-functions
package
#2521
Conversation
Hi @aaronpowell, Welcome, and thank you for contributing to Remix! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run. Thanks! - The Remix team |
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
2e6ce40
to
9537e52
Compare
@aaronpowell Could you please rebase your branch on latest Also make sure to not use |
remix-azure-functions
package
@MichaelDeBoey yep, I'll clean up with a rebase shortly and refactor it to follow the patterns of the other adapters. My first goal was just to get the build passing, then tidy up 😅 |
dff4cfd
to
a8db21c
Compare
@aaronpowell I think some things went wrong with rebasing. If you can't figure it out, you could do git reset --hard dev
git cherry-pick [commit-sha] You need to do |
@MichaelDeBoey as yes, I see a few errors across the style and jest files. It's a little difficult to do the cherry picking as there's 30 commits that need to be brought in on the HEAD for the rebase and they are from pre-remix 1.0 so they have some differences that can be difficult to manually merge. I can clean up those files on the HEAD now. |
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.
@aaronpowell Also make sure to add .eslintrc
to the template and update the favicon to the new .ico
instead of the old .png
please
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.
Could you please also rebase onto latest dev
& resolve conflicts?
I'll get to a rebase today and hopefully not stuff up any of the merges this time |
62a6f22
to
aa8dab7
Compare
Deployed using the template - https://agreeable-desert-0f0e5cc10.1.azurestaticapps.net/ |
@aaronpowell Could you please run the following to make sure we're not breaking git checkout dev yarn.lock && yarn clean && yarn |
0d07670
to
33f724a
Compare
Doing my best on the older rebases were harder as they were quite out of date. The last commit I had forgot to run a |
@aaronpowell Please rebase your branch onto latest |
33f724a
to
b807f4b
Compare
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
Co-authored-by: Michaël De Boey <info@michaeldeboey.be> Co-authored-by: Christian Köberl <derkoe@users.noreply.github.com>
253e537
to
8e94a44
Compare
For all people not patient enough (like me): I have published the package here: https://www.npmjs.com/package/@derkoe/remix-azure-functions. (The build/code publishing this package is here: https://github.com/derkoe/remix-azure-functions) |
could someone give us ETA on this one? We are keen to use it and the sample GitHub/npm package is not quite clear on how to use it. |
@mzaatar The team will look into this PR whenever they have the time for it, but new adapters are not high on the priority list for the moment. |
@mzaatar I have a working demo here https://github.com/derkoe/remix-azure-functions-demo using the npm package @derkoe/remix-azure-functions. |
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.
It seems like we're also missing remix.env.d.ts
in the template 🤔
@@ -0,0 +1,9 @@ | |||
/* eslint-disable import/no-extraneous-dependencies */ |
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.
We now have a new way of handling magicExports
.
Please add the magic exports in getMagicExports
function of the root's rollup.utils.js
"@azure/static-web-apps-cli": "^0.8.3", | ||
"@remix-run/dev": "*", | ||
"@remix-run/eslint-config": "*", | ||
"@remix-run/serve": "*", |
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.
Not needed I think, as we already have @azure/functions
?
"@remix-run/serve": "*", |
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.
The package @azure/functions
isn't a hosting package, it's just a types package.
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.
Then this one should go into dependencies
I think?
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
We aren't looking to add new adapter packages for the time being, though we encourage hosts to maintain and promote them! If anything changes on that front we will reopen this. |
@aaronpowell If you are planning to update this, could this be done using functions v4? |
No, I don't have any intents to update this PR. |
This revives #246 and uplifts it to the current release of remix.