Skip to content

fix(functions): insert underscore between a digit and a capital letter in idToSnakeCaseFast#1416

Open
sarmah-rup wants to merge 1 commit into
jitsucom:newjitsufrom
sarmah-rup:fix/snakecase-digit-boundary
Open

fix(functions): insert underscore between a digit and a capital letter in idToSnakeCaseFast#1416
sarmah-rup wants to merge 1 commit into
jitsucom:newjitsufrom
sarmah-rup:fix/snakecase-digit-boundary

Conversation

@sarmah-rup

Copy link
Copy Markdown

What

idToSnakeCaseFast in libs/functions/src/lib/strings.ts does not insert an underscore between a digit and a following capital letter. An id like field1Name becomes field1name instead of field1_name.

Why I hit this

I run events into a warehouse through Jitsu and noticed some of my columns came out mis-named. Properties like field1Name, address2Line and plan9FromOuterSpace landed as field1name, address2line and plan9from_outer_space rather than the snake_case I expected. That breaks downstream models that reference field1_name.

Root cause

In the loop, needUnderscore is only turned on after a latin letter, so the digit to capital boundary is missed:

// we add underscore only between latin letters
needUnderscore = (c >= aCode && c <= zCode) || (c >= ACode && c <= ZCode);

The comment even says "only between latin letters", but the canonical reference implementation idToSnakeCaseRegex in libs/core-functions-lib/src/functions/lib/strings.ts uses a (?<=[a-zA-Z0-9]) lookbehind, which includes digits. So the fast and regex versions disagree exactly on digit boundaries.

Fix

Include the digit range in needUnderscore, reusing the zeroCode / nineCode constants already declared at the top of the file:

needUnderscore = (c >= aCode && c <= zCode) || (c >= ACode && c <= ZCode) || (c >= zeroCode && c <= nineCode);

Tests

Added regression cases to libs/core-functions-lib/__tests__/strings.test.ts asserting the corrected output (field1Name becomes field1_name, address2Line becomes address2_line, plan9FromOuterSpace becomes plan9_from_outer_space) and that idToSnakeCaseFast now matches idToSnakeCaseRegex. The two new tests fail on newjitsu before the change and pass after. The full core-functions-lib suite is green at 42 tests.

…akeCaseFast

idToSnakeCaseFast only set needUnderscore after a latin letter, so an
identifier like field1Name became field1name instead of field1_name. The
reference idToSnakeCaseRegex uses a [a-zA-Z0-9] lookbehind and includes
digits, so the two diverged on digit boundaries.

Include the digit range in needUnderscore using the existing zeroCode and
nineCode constants, and add regression tests co-validating the fast and
regex implementations.
@vklimontovich

Copy link
Copy Markdown
Contributor

Hey @sarmah-rup, thank you for the PR! If we merge it, would it mean that columns containing digits will be renamed? May be too disruptive

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.

2 participants