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: add @remix-run/azure-functions package #2521

Closed
wants to merge 68 commits into from

Conversation

aaronpowell
Copy link

@aaronpowell aaronpowell commented Mar 28, 2022

This revives #246 and uplifts it to the current release of remix.

  • Docs
  • Tests

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Mar 28, 2022

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 CLA Signed label will be added to the pull request.

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

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Mar 28, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@aaronpowell aaronpowell changed the title Azure Functions hosting for Remix feat(remix-azure): azure functions hosting Mar 28, 2022
@MichaelDeBoey
Copy link
Member

@aaronpowell Could you please rebase your branch on latest dev?

Also make sure to not use @remix-run/server-runtime directly, but use @remix-run/node instead in the adapters.
Check out #2359 to see how all other adapters have changed.

@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label Mar 29, 2022
@MichaelDeBoey MichaelDeBoey changed the title feat(remix-azure): azure functions hosting feat: add remix-azure-functions package Mar 29, 2022
@aaronpowell
Copy link
Author

@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 😅

@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Mar 29, 2022
@MichaelDeBoey
Copy link
Member

@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 git push --force in order to make the rebase complete.

@aaronpowell
Copy link
Author

@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.

Copy link
Member

@MichaelDeBoey MichaelDeBoey left a 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

jest.config.js Outdated Show resolved Hide resolved
packages/remix-azure-functions/server.ts Outdated Show resolved Hide resolved
packages/remix-azure-functions/server.ts Outdated Show resolved Hide resolved
packages/remix-azure-functions/server.ts Outdated Show resolved Hide resolved
packages/remix-azure-functions/server.ts Outdated Show resolved Hide resolved
packages/remix-server-runtime/headers.ts Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
templates/azure-functions/.gitignore Show resolved Hide resolved
packages/remix-azure-functions/package.json Outdated Show resolved Hide resolved
templates/azure-functions/.eslintrc Outdated Show resolved Hide resolved
packages/remix-azure-functions/__tests__/server-test.ts Outdated Show resolved Hide resolved
packages/remix-azure-functions/server.ts Outdated Show resolved Hide resolved
@aaronpowell aaronpowell marked this pull request as ready for review March 30, 2022 05:17
package.json Outdated Show resolved Hide resolved
packages/remix-architect/package.json Outdated Show resolved Hide resolved
packages/remix-server-runtime/package.json Outdated Show resolved Hide resolved
packages/remix-vercel/package.json Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
templates/azure-functions/package.json Outdated Show resolved Hide resolved
templates/azure-functions/package.json Show resolved Hide resolved
templates/azure-functions/package.json Outdated Show resolved Hide resolved
packages/remix-azure-functions/__tests__/server-test.ts Outdated Show resolved Hide resolved
packages/remix-azure-functions/package.json Outdated Show resolved Hide resolved
Copy link
Member

@MichaelDeBoey MichaelDeBoey left a 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?

docs/other-api/adapter.md Outdated Show resolved Hide resolved
packages/remix-azure-functions/package.json Outdated Show resolved Hide resolved
packages/remix-azure-functions/server.ts Outdated Show resolved Hide resolved
packages/remix-azure-functions/server.ts Outdated Show resolved Hide resolved
@aaronpowell
Copy link
Author

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

@aaronpowell
Copy link
Author

Deployed using the template - https://agreeable-desert-0f0e5cc10.1.azurestaticapps.net/

@MichaelDeBoey
Copy link
Member

@aaronpowell Could you please run the following to make sure we're not breaking yarn.lock with the rebases?

git checkout dev yarn.lock && yarn clean && yarn

@aaronpowell
Copy link
Author

@aaronpowell Could you please run the following to make sure we're not breaking yarn.lock with the rebases?

git checkout dev yarn.lock && yarn clean && yarn

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 yarn clean && yarn, it wasn't until I pushed the commit prior that I realised and fixed it up

@MichaelDeBoey
Copy link
Member

@aaronpowell Please rebase your branch onto latest dev & resolve conflicts.

@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label May 3, 2022
@derkoe
Copy link

derkoe commented Aug 21, 2022

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)

@mzaatar
Copy link

mzaatar commented Sep 20, 2022

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.

@MichaelDeBoey
Copy link
Member

@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.

@derkoe
Copy link

derkoe commented Sep 20, 2022

@mzaatar I have a working demo here https://github.com/derkoe/remix-azure-functions-demo using the npm package @derkoe/remix-azure-functions.

@MichaelDeBoey MichaelDeBoey added the needs-response We need a response from the original author about this issue/PR label Sep 29, 2022
Copy link
Member

@MichaelDeBoey MichaelDeBoey left a 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 🤔

templates/azure-functions/public/staticwebapp.config.json Outdated Show resolved Hide resolved
@@ -0,0 +1,9 @@
/* eslint-disable import/no-extraneous-dependencies */
Copy link
Member

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

packages/remix-azure-functions/package.json Outdated Show resolved Hide resolved
packages/remix-azure-functions/package.json Outdated Show resolved Hide resolved
templates/azure-functions/app/entry.server.tsx Outdated Show resolved Hide resolved
templates/azure-functions/package.json Outdated Show resolved Hide resolved
templates/azure-functions/package.json Outdated Show resolved Hide resolved
"@azure/static-web-apps-cli": "^0.8.3",
"@remix-run/dev": "*",
"@remix-run/eslint-config": "*",
"@remix-run/serve": "*",
Copy link
Member

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?

Suggested change
"@remix-run/serve": "*",

Copy link
Author

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.

Copy link
Member

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?

templates/azure-functions/package.json Outdated Show resolved Hide resolved
templates/azure-functions/tsconfig.json Show resolved Hide resolved
Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
@github-actions github-actions bot removed the needs-response We need a response from the original author about this issue/PR label Nov 3, 2022
@chaance
Copy link
Collaborator

chaance commented Jan 23, 2023

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.

@chaance chaance closed this Jan 23, 2023
@arunmmanoharan
Copy link

@aaronpowell If you are planning to update this, could this be done using functions v4?

@aaronpowell
Copy link
Author

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants