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

refactor: [M3-8247] - Remove ramda from Utilities #11861

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

harsh-akamai
Copy link
Contributor

Description 📝

Ramda should be completely removed/replaced with native JS equivalent

Changes 🔄

Removed Ramda 🧹 from :

  • utilities/createStringsFromDevices.ts
  • utilities/creditCard.test.ts
  • utilities/creditCard.ts
  • utilities/formikErrorUtils.ts
  • utilities/getAll.ts
  • utilities/getSelectedOptionFromGroupedOptions.test.ts
  • utilities/getSelectedOptionFromGroupedOptions.ts
  • utilities/initWindows.ts
  • utilities/isNilOrEmpty.ts
  • utilities/testHelpers.tsx

How to test 🧪

Verification steps

  • Ensure no existing functionality is broken
Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

@harsh-akamai harsh-akamai self-assigned this Mar 17, 2025
@harsh-akamai harsh-akamai marked this pull request as ready for review March 17, 2025 13:43
@harsh-akamai harsh-akamai requested a review from a team as a code owner March 17, 2025 13:43
@harsh-akamai harsh-akamai requested review from bnussman-akamai and coliu-akamai and removed request for a team March 17, 2025 13:43
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this file since the function is not being imported anywhere in CM.

Copy link

github-actions bot commented Mar 18, 2025

Coverage Report:
Base Coverage: 79.81%
Current Coverage: 79.84%

@linode-gh-bot
Copy link
Collaborator

Cloud Manager UI test results

🎉 540 passing tests on test run #7 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing540 Passing3 Skipped113m 42s

Copy link
Contributor

@coliu-akamai coliu-akamai left a comment

Choose a reason for hiding this comment

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

thanks @harsh-akamai! Left a few comments for now + I also need to look at mergeRight a bit more and wrap my head around the examples 😅 - appreciate your test cases!

import { isEmpty, isNil } from 'ramda';

export const isNilOrEmpty = (v: any) => isNil(v) || isEmpty(v);
export const isNilOrEmpty = (v: null | number | object | string | undefined) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

wondering if we should keep the input type as any - unless we're trying to explicitly exclude stuff like booleans, etc (which makes sense, we probably wouldn't call isNilOrEmpty(true))


export const isNilOrEmpty = (v: any) => isNil(v) || isEmpty(v);
export const isNilOrEmpty = (v: null | number | object | string | undefined) =>
v == null || // covers for null and undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's functionally the same, but I would prefer v === null || v === undefined for more clarity

it('should return true if variable is null or undefined or empty object', () => {
const x = null;
const y = undefined;
const obj = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

could we add test cases for other types of objects like arrays, sets, etc

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.

3 participants