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

chore: update @architect/functions #7207

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

MichaelDeBoey
Copy link
Member

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Aug 21, 2023

⚠️ No Changeset found

Latest commit: f49522a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@brophdawg11 brophdawg11 removed their request for review August 22, 2023 15:04
@MichaelDeBoey MichaelDeBoey force-pushed the update-architect__functions branch 7 times, most recently from 907bb42 to 71ab774 Compare August 29, 2023 02:14
@MichaelDeBoey MichaelDeBoey force-pushed the update-architect__functions branch 3 times, most recently from b76c6fc to de56318 Compare August 31, 2023 14:28
@MichaelDeBoey MichaelDeBoey force-pushed the update-architect__functions branch 2 times, most recently from 06694a6 to 0a9cddb Compare September 6, 2023 19:40
@brophdawg11
Copy link
Contributor

I don't know much about architect - but if this is a major version upgrade of the dep - does that mean breaking changes to the Remix surface area?

@lpsinger
Copy link
Contributor

lpsinger commented Feb 5, 2024

I don't know much about architect - but if this is a major version upgrade of the dep - does that mean breaking changes to the Remix surface area?

No.

Although the latest version of this dependency is now 8.0.0. @MichaelDeBoey, would you please update your PR to @architect/functions 8.0.0?

@MichaelDeBoey
Copy link
Member Author

@lpsinger I've updated this PR to use latest version of @architect/functions now

@MichaelDeBoey MichaelDeBoey force-pushed the update-architect__functions branch 2 times, most recently from faedd00 to 0b0fa27 Compare June 3, 2024 11:10
@lpsinger
Copy link
Contributor

lpsinger commented Jun 3, 2024

Could you change it to 7.x? @architect/functions 8.x switched from using the official AWS SDK to their own aws-lite client, which I am skeptical of.

@MichaelDeBoey
Copy link
Member Author

@lpsinger Can you explain why you're skeptical about the change?
The architect team seems to recommend it now, so I think we should update to their recommendations as well

Do you think those changes can break existing Remix apps?

@lpsinger
Copy link
Contributor

lpsinger commented Jun 3, 2024

aws-lite is supposed to have quicker cold-start times than @aws-sdk/, but all but the simplest projects are bound to have dependencies on @aws-sdk/ anyway through other projects that depend upon it. Since now we have to spin up both clients, aws-lite is going to make our cold-start times strictly longer.

Also, Architect has made some changes that may have dangerous consequences for my project (see architect/architect#1483 (comment)), and we are stuck with using an old version of Architect until those issues are fixed.

If Remix requires @architect/functions 8.x, then my project may not be able to track the latest version of Remix either.

@MichaelDeBoey
Copy link
Member Author

@lpsinger I've reverted to v7.x again 👍

@lpsinger
Copy link
Contributor

lpsinger commented Jun 3, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants