-
Notifications
You must be signed in to change notification settings - Fork 149
Upgrade dependencies to solve audits #798
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
Conversation
|
All checks passed. Can @jamonholmgren review it and merge it? |
src/toolbox/string-tools.ts
Outdated
| trimStart, | ||
| upperCase, | ||
| upperFirst, | ||
| } from 'lodash' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change these to / imports for the sake of bundle size?
i.e.
import camelCase from 'lodash/camelCase'
import kebabCase from 'lodash/kebabCase'
import lowerCase from 'lodash/lowerCase'
...
etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if something special needs to be set, but importing this way does not work camelCase/kebabCase/lowerCase are undefined this way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robinheinze it works using require.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were you using import {} with curly braces when it was undefined or was it a default import like `import camelCase from 'lodash/camelCase'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were you using
import {}with curly braces when it was undefined or was it a default import like `import camelCase from 'lodash/camelCase'?
Here is the original implementation with your suggestions https://github.com/infinitered/gluegun/compare/c4a4951fb34da11c9529be97de4e725596ec927b..d143665521aae30996eb79d25c36637a93381334 , and here is the output during tests https://app.circleci.com/pipelines/github/infinitered/gluegun/1273/workflows/d48838a4-b033-4f3b-a23b-b8265f0a304a/jobs/2230
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's just stick with require then`. No problem.
cc70313 to
c4a4951
Compare
* Upgrade to fix vulnerabilities * Regenerate print-tools.test.ts * Fix lodash imports --------- Co-authored-by: Maryna Boieva <[email protected]> Co-authored-by: gulanf <[email protected]>
## [5.2.1](v5.2.0...v5.2.1) (2025-11-21) ### Bug Fixes * Upgrade dependencies to solve audits ([#798](#798)) ([ec2541e](ec2541e))
Fixed unit tests from #796