Skip to content

feat(crypto-fips): add migration recipe for crypto.fips #177

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 34 commits into from
Oct 27, 2025
Merged

feat(crypto-fips): add migration recipe for crypto.fips #177

merged 34 commits into from
Oct 27, 2025

Conversation

@max-programming
Copy link
Contributor

@max-programming max-programming commented Aug 13, 2025

Closes #175

@max-programming
feat(crypto-fips): add migration recipe for transitioning from crypto…
39f92b0 
….fips to crypto.getFips() and crypto.setFips()
@max-programming
Copy link
Contributor Author

max-programming commented Aug 13, 2025

Let me know if the settings.json change feels unnecessary, if that's the case, I'll remove it

Personally, I think having that helps

@avivkeller
Copy link
Member

avivkeller commented Aug 13, 2025

Let me know if the settings.json change feels unnecessary, if that's the case, I'll remove it

Personally, I think having that helps

You don't have to remove it entirely, but can it be a separate PR (to keep our commits organized)?

Thank you!

avivkeller
avivkeller approved these changes Aug 13, 2025
View reviewed changes
@max-programming
Copy link
Contributor Author

max-programming commented Aug 13, 2025

@avivkeller Alright then. I'm gonna make another PR for another recipe where I'll include this change as a separate commit

@max-programming
chore(vscode): revert settings json file to default
17c3bc9
@max-programming
refactor(crypto-fips): streamline crypto base path retrieval and impr…
078fecd 
…ove code readability
@max-programming
refactor(crypto-fips): use generators for crypto base path collection
7cd8777 
Replaces array concatenation with generator-based iteration
to improve readability, memory usage, and performance.
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy approved these changes Aug 13, 2025
View reviewed changes
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WOW 😵‍💫 that's an awesome first contribution. I love your usage of yield.

Thanks you for your contribution, I hope that you had enjoyed it. If you have any feedback feel it

@max-programming
Copy link
Contributor Author

max-programming commented Aug 13, 2025

Thanks @AugustinMauroy!

I'll update the suggested changes
Also there's one two more test cases that are failing which I'll add and re-request a review

@max-programming
refactor(crypto-fips): implement transformation for crypto.fips to ge…
b4d9c0c 
…tFips/setFips

This update introduces a transformation for replacing deprecated `crypto.fips` calls with `crypto.getFips()` and `crypto.setFips()`. It includes enhancements to variable collection, import specifier updates, and destructuring adjustments to ensure proper usage of the new methods. Additionally, new test cases have been added to validate the changes.
@max-programming
Copy link
Contributor Author

max-programming commented Aug 13, 2025

@AugustinMauroy I think some functions I used can be put into utils. Ones that are used to get import specifiers and all
Let me know which ones can be re-used in other codemods

AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
AugustinMauroy
AugustinMauroy reviewed Aug 13, 2025
View reviewed changes
@max-programming
Copy link
Contributor Author

max-programming commented Aug 13, 2025

For the utility functions thing, I'll examine those and probably open another PR for that specifically

@max-programming
Copy link
Contributor Author

max-programming commented Oct 19, 2025 •
edited
Loading

Not sure why these checks fail

How can we get them to pass?

@AugustinMauroy
Copy link
Member

AugustinMauroy commented Oct 19, 2025

Not sure why these checks fail

How can we get them to pass?

because you touch to the utility and you break something

@max-programming
Copy link
Contributor Author

max-programming commented Oct 19, 2025

Ah I think I found the failing part

@max-programming
Copy link
Contributor Author

max-programming commented Oct 19, 2025

Passed finally

This was such a headache lol

@JakobJingleheimer
Copy link
Member

JakobJingleheimer commented Oct 21, 2025

@max-programming thanks for this and getting it updated! I'll try to get to the review tomorrow.

JakobJingleheimer
JakobJingleheimer requested changes Oct 23, 2025
View reviewed changes
Copy link
Member

@JakobJingleheimer JakobJingleheimer left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good! It needs to support dynamic imports—we have utilities to help with that (I referenced them in some code suggestions) and tests for them; I think that will be pretty quick/simple to include. Good to go IMO once those are in 🙂

@JakobJingleheimer JakobJingleheimer added missing functionality Required use-case missing awaiting author Reviewer has requested something from the author and removed awaiting reviewer Author has responded and needs action from the reviewer labels Oct 23, 2025
@max-programming
Copy link
Contributor Author

max-programming commented Oct 24, 2025

@JakobJingleheimer Made the changes! Enabled dynamic imports (it was quite simple lol)

Also be sure to verify this settings.json change

I did it because generally my default formatter is Prettier but this project recommends Biome

brunocroh
brunocroh approved these changes Oct 25, 2025
View reviewed changes
JakobJingleheimer
JakobJingleheimer approved these changes Oct 26, 2025
View reviewed changes
Copy link
Member

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌 Thanks!

@JakobJingleheimer JakobJingleheimer removed missing functionality Required use-case missing awaiting author Reviewer has requested something from the author labels Oct 26, 2025
@brunocroh
Copy link
Member

brunocroh commented Oct 27, 2025

Thank you so much for your contribution @max-programming, You did a great job, we really appreciate it!

@brunocroh brunocroh merged commit 641d544 into nodejs:main Oct 27, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AugustinMauroy AugustinMauroy AugustinMauroy approved these changes

@JakobJingleheimer JakobJingleheimer JakobJingleheimer approved these changes

@brunocroh brunocroh brunocroh approved these changes

@avivkeller avivkeller Awaiting requested review from avivkeller

Assignees

No one assigned

Labels

dep:v23 Migration handles deprecation introduced in node v23

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: DEP0093: crypto.fips is deprecated and replaced

5 participants

@max-programming @avivkeller @AugustinMauroy @JakobJingleheimer @brunocroh