Skip to content

Conversation

RobbieTheWagner
Copy link
Member

๐Ÿ“Œ Summary

This PR drops the 2.x version of luxon from dependencies.

๐Ÿ› ๏ธ Detailed description

I think the way it was defined consumers would always get 2.x, which has security vulnerabilities. If we do need to support 2.x, we could also make it a peerDep instead of dropping it, but I am not sure if there is a reason we need 2.x support or not.

๐Ÿ“ธ Screenshots

๐Ÿ”— External links

Jira ticket: HDS-XXX
Figma file: [if it applies]


๐Ÿ‘€ Component checklist

๐Ÿ’ฌ Please consider using conventional comments when reviewing this PR.

@RobbieTheWagner RobbieTheWagner requested a review from a team as a code owner April 23, 2025 11:44
Copy link

vercel bot commented Apr 23, 2025

The latest updates on your projects. Learn more about Vercel for Git โ†—๏ธŽ

Name Status Preview Updated (UTC)
hds-showcase โœ… Ready (Inspect) Visit Preview Apr 23, 2025 1:01pm
hds-website โœ… Ready (Inspect) Visit Preview Apr 23, 2025 1:01pm

@didoo
Copy link
Contributor

didoo commented Apr 23, 2025

@RobbieTheWagner for reference: #2515 (comment) and #2515 (comment)

/cc @alex-ju

@RobbieTheWagner
Copy link
Member Author

@didoo I took a look at those comments and atlas does not have luxon installed at all, so I am confused. I think the correct paths forward here are either just use 3.x like this PR proposes or we could make it a peerDep and allow whatever versions, but currently if anyone installs HDS and they do not specify their own version of luxon, this is forcing them to use 2.x which has vulnerabilities.

@didoo
Copy link
Contributor

didoo commented Apr 23, 2025

@didoo I took a look at those comments and atlas does not have luxon installed at all, so I am confused. I think the correct paths forward here are either just use 3.x like this PR proposes or we could make it a peerDep and allow whatever versions, but currently if anyone installs HDS and they do not specify their own version of luxon, this is forcing them to use 2.x which has vulnerabilities.

@alex-ju and @KristinLBradley worked on this and I'm sure have more context; would be a problem to wait for when they're back from PTO next week?

@RobbieTheWagner
Copy link
Member Author

@alex-ju and @KristinLBradley worked on this and I'm sure have more context; would be a problem to wait for when they're back from PTO next week?

@didoo I think waiting should be fine, but I am not sure. I don't know what our requirements are for a timeline for having all the vulnerabilities resolved in atlas though. If the main reason we were trying to support 2.x was for atlas, I don't think we need it anymore, and in the meantime we are forcing all consumers of HDS to install the old 2.x version with the vulnerabilities, since this is not a peerDep.

@RobbieTheWagner
Copy link
Member Author

@didoo do you still think we should wait to merge this or are the approvals from @zamoore and @MelSumner sufficient to merge?

@didoo
Copy link
Contributor

didoo commented Apr 23, 2025

@didoo do you still think we should wait to merge this or are the approvals from @zamoore and @MelSumner sufficient to merge?

@RobbieTheWagner we discussed this today in our HDS Engineering sync, and if @MelSumner and @zamoore approve, it's OK to merge. ๐Ÿ‘

@RobbieTheWagner RobbieTheWagner merged commit d9bc3b5 into main Apr 24, 2025
16 checks passed
@RobbieTheWagner RobbieTheWagner deleted the luxon-3 branch April 24, 2025 13:29
@hashibot-hds hashibot-hds mentioned this pull request Apr 22, 2025
@alex-ju alex-ju added this to the [email protected] milestone May 7, 2025
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.

6 participants