Skip to content

Remove camelcase-keys, replace with inline utility#7194

Merged
ryancbahan merged 2 commits intomainfrom
04-03-04-03-remove-camelcase-keys
Apr 7, 2026
Merged

Remove camelcase-keys, replace with inline utility#7194
ryancbahan merged 2 commits intomainfrom
04-03-04-03-remove-camelcase-keys

Conversation

@ryancbahan
Copy link
Copy Markdown
Contributor

@ryancbahan ryancbahan commented Apr 3, 2026

Summary

  • Add camelcase-keys.ts (~40 lines) in app-logs/ with parity behavior to the npm package
  • Handles: snake_case, kebab-case, deep recursion, leading underscore stripping, ALL_CAPS normalization, Date/non-plain-object preservation
  • Update 4 import sites (poll-app-logs.ts, utils.ts, and their test files)
  • Remove camelcase-keys from packages/app/package.json

Test plan

  • 16 parity tests covering all edge cases
  • All 57 app-logs tests pass

@ryancbahan ryancbahan force-pushed the 04-03-04-03-remove-camelcase-keys branch from 1a01cd0 to ffadb6c Compare April 3, 2026 22:37
@ryancbahan ryancbahan marked this pull request as ready for review April 3, 2026 22:39
@ryancbahan ryancbahan requested a review from a team as a code owner April 3, 2026 22:39
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

/**
* Converts a string from snake_case or kebab-case to camelCase.
*/
function toCamelCase(str: string): string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We already have a similar function in cli-kit. If you need anything else, I think we should add it to that file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Happy to follow up on this, but i think we should prioritize shipping this to support the supply chain remediation project

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, camelcase-keys is pinned to a specific version that we have been using for a long time, so it doesn't look urgent.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with Gonzalo here, we can use the one from cli-kit. No need to add 2 new files with 100+ changes for something that already exists :thinking_face:
An agent can quickly update this PR to use the camelize function from cli-kit, I don't think there is that much urgency to remove this dependency that prevents us to doing it the right way!

Copy link
Copy Markdown
Contributor Author

@ryancbahan ryancbahan Apr 7, 2026

Choose a reason for hiding this comment

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

I'll update this to use camelize. The urgency here is founded IMO -- the js community has had two major supply chain attacks in two weeks. Removing deps is a company-wide effort right now, and the cost of consolidation is cheap in comparison. That being said, it's fair feedback that we can just do the centralization now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@gonzaloriestra true, but we don't always use --frozen-lockfile and minor version resolutions can transitively lead to major version changes in other deps

@ryancbahan ryancbahan changed the base branch from 04-03-04-03-remove-ansi-colors to graphite-base/7194 April 6, 2026 21:55
@ryancbahan ryancbahan force-pushed the graphite-base/7194 branch from 0f7fea3 to afe68a1 Compare April 7, 2026 13:50
@ryancbahan ryancbahan force-pushed the 04-03-04-03-remove-camelcase-keys branch from ffadb6c to 806663c Compare April 7, 2026 13:50
@ryancbahan ryancbahan changed the base branch from graphite-base/7194 to main April 7, 2026 13:50
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/monorail.d.ts
@@ -66,10 +66,6 @@ export interface Schemas {
             cmd_app_linked_config_uses_cli_managed_urls?: Optional<boolean>;
             cmd_app_warning_api_key_deprecation_displayed?: Optional<boolean>;
             cmd_app_deployment_mode?: Optional<string>;
-            cmd_app_validate_json?: Optional<boolean>;
-            cmd_app_validate_valid?: Optional<boolean>;
-            cmd_app_validate_issue_count?: Optional<number>;
-            cmd_app_validate_file_count?: Optional<number>;
             cmd_dev_tunnel_type?: Optional<string>;
             cmd_dev_tunnel_custom_hash?: Optional<string>;
             cmd_dev_urls_updated?: Optional<boolean>;

ryancbahan and others added 2 commits April 7, 2026 07:57
Add a local camelcase-keys.ts with parity behavior: snake_case/kebab-case
to camelCase key conversion, deep recursion, leading underscore stripping,
ALL_CAPS handling, and Date/non-plain-object preservation. Includes 16
parity tests. Update 4 call sites in app-logs to use the local module.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ryancbahan ryancbahan force-pushed the 04-03-04-03-remove-camelcase-keys branch from 806663c to da3245f Compare April 7, 2026 13:57
@ryancbahan ryancbahan added this pull request to the merge queue Apr 7, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 7, 2026
@ryancbahan ryancbahan added this pull request to the merge queue Apr 7, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 7, 2026
@ryancbahan ryancbahan added this pull request to the merge queue Apr 7, 2026
Merged via the queue into main with commit a66b1c7 Apr 7, 2026
123 of 130 checks passed
@ryancbahan ryancbahan deleted the 04-03-04-03-remove-camelcase-keys branch April 7, 2026 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants