Skip to content

common: export lodash in common #1225

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

Open
wants to merge 5 commits into
base: epic/common-3
Choose a base branch
from

Conversation

hunterachieng
Copy link
Contributor

@hunterachieng hunterachieng commented Jun 5, 2025

Summary

Export lodash in common

Fixes #831

Details

Add technical details of what you've changed (and why).

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to
know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our
Responsible AI Policy

Review Checklist

Before merging, the reviewer should check the following items:

  • Does the PR do what it claims to do?
  • If this is a new adaptor, added the adaptor on marketing website ?
  • If this PR includes breaking changes, do we need to update any jobs in
    production? Is it safe to release?
  • Are there any unit tests?
  • Is there a changeset associated with this PR? Should there be? Note that
    dev only changes don't need a changeset.
  • Have you ticked a box under AI Usage?

Signed-off-by: Hunter Achieng <[email protected]>
Signed-off-by: Hunter Achieng <[email protected]>
Signed-off-by: Hunter Achieng <[email protected]>
* @see https://lodash.com/docs/
* @module lodash
*
* @example <caption>Split an array into chunks of 2 items each</caption>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is excellent 👏

@josephjclark josephjclark changed the base branch from main to epic/common-3 June 6, 2025 08:28
@josephjclark josephjclark changed the base branch from epic/common-3 to main June 6, 2025 08:29
@josephjclark josephjclark changed the base branch from main to epic/common-3 June 6, 2025 08:30
@josephjclark
Copy link
Collaborator

@hunterachieng have you tested this in the CLI? I don't think the export is right

I think you've exported it to util, so you have to do util._.pick(), which is wierd 🤔

I've also run a docs build here and _ hasn't been picked up at all.

What I think we need to end up with is this:
image

Which is a bit weird isn't it? So maybe we can make it _.* (lodash)

But the point is that we're exporting a namespace. We're not documenting the whole namespace here, but we do need something to click on to get to our docs.

This might be a bit tricky to fix so maybe we can catch up with this on Monday.

@hunterachieng
Copy link
Contributor Author

@josephjclark Let me try to fix it. I did use the same method we used with throwError but let me iterate

@hunterachieng
Copy link
Contributor Author

@josephjclark I have tried redoing it and exporting it away from utils. I am getting that _ is not defined. I have pushed the latest iteration

Signed-off-by: Hunter Achieng <[email protected]>
Signed-off-by: Hunter Achieng <[email protected]>
@josephjclark
Copy link
Collaborator

Hi @hunterachieng - I'll try and get this wrapped up for you this evening

@josephjclark
Copy link
Collaborator

@hunterachieng I've created a branch lodash-2 where:

  1. exports work
  2. documentation is a little bit better

image

I think what we need to do is work out how to suppress the signature in the docs, and then that might be good enough?

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

Successfully merging this pull request may close these issues.

Add util.indexOf() function in common
2 participants