Skip to content

Replace lodash with es-toolkit #197

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.

Sign up for GitHub

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

Draft
myarmolinsky wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Replace lodash with es-toolkit #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
"dependencies": {
"@balena/abstract-sql-compiler": "^11.2.0",
"@balena/odata-parser": "^4.2.6",
"@types/lodash": "^4.17.20",
"@types/memoizee": "^0.4.12",
"@types/string-hash": "^1.1.3",
"lodash": "^4.17.21",
"es-toolkit": "^1.43.0",
"memoizee": "^0.4.17",
"string-hash": "^1.1.3"
},
Expand Down
181 changes: 103 additions & 78 deletions src/odata-to-abstract-sql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import _ from 'lodash';
import {
differenceWith,
isEqual,
capitalize,
pick,
omitBy,
isNil,
} from 'es-toolkit';
import { get } from 'es-toolkit/compat';
import memoize from 'memoizee';
import stringHash from 'string-hash';
import {
Expand Down Expand Up @@ -433,20 +441,21 @@ export class OData2AbstractSQL {
if (minimizeAliases === false && aliasLength <= MAX_ALIAS_LENGTH) {
return alias;
}
alias = _(alias.toLowerCase())
alias = alias
.toLowerCase()
.split('.')
.map((part) => {
if (minimizeAliases === false && aliasLength <= MAX_ALIAS_LENGTH) {
return part;
}
aliasLength -= part.length;
const shortAlias = _(part)
const shortAlias = part
.split('-')
.map((part2) => {
part2 = _(part2)
part2 = part2
.split(' ')
.map((part3) => {
part3 = _(part3)
part3 = part3
.split('$')
.map((part4) => shortAliases[part4] ?? part4)
.join('$');
Expand Down Expand Up @@ -500,7 +509,7 @@ export class OData2AbstractSQL {
this.reset();
this.bindVarsLength = bindVarsLength;
let tree: AbstractSqlQuery;
if (_.isEmpty(path)) {
if (Object.keys(path).length === 0) {
tree = ['$serviceroot'];
} else if (['$metadata', '$serviceroot'].includes(path.resource)) {
tree = [path.resource];
Expand Down Expand Up @@ -908,7 +917,7 @@ export class OData2AbstractSQL {
((index.type === 'UNIQUE' && index.predicate == null) ||
index.type === 'PRIMARY KEY') &&
sqlFieldNames.length === index.fields.length &&
_.isEqual(index.fields.slice().sort(), sqlFieldNames)
isEqual(index.fields.slice().sort(), sqlFieldNames)
);
})
) {
Expand Down Expand Up @@ -1073,12 +1082,11 @@ export class OData2AbstractSQL {
`Could not resolve relationship for '${resourceName}'`,
);
}
const relationshipPath = _(relationship)
const relationshipPath = relationship
.split('__')
.map(odataNameToSqlName)
.flatMap((sqlName) => this.Synonym(sqlName).split('-'))
.value();
const relationshipMapping = _.get(resourceRelations, relationshipPath);
.flatMap((sqlName) => this.Synonym(sqlName).split('-'));
const relationshipMapping = get(resourceRelations, relationshipPath);
if (!relationshipMapping?.$) {
throw new SyntaxError(
`Could not resolve relationship mapping from '${resourceName}' to '${relationshipPath}'`,
Expand Down Expand Up @@ -1110,10 +1118,10 @@ export class OData2AbstractSQL {
sqlNameToODataName(field.fieldName),
]);
}
const fields = _.differenceWith(
const fields = differenceWith(
odataFieldNames,
query.select,
(a, b) => a[1] === _.last(b),
(a, b) => a[1] === b.at(-1),
).map((args) => this.AliasSelectField(...args));
query.select = query.select.concat(fields);
}
Expand Down Expand Up @@ -1195,7 +1203,7 @@ export class OData2AbstractSQL {
case 'and':
case 'or':
return [
_.capitalize(type),
capitalize(type),
...rest.map((v) => this.BooleanMatch(v)),
];
case 'not': {
Expand Down Expand Up @@ -1270,7 +1278,7 @@ export class OData2AbstractSQL {
throw new SyntaxError('Unexpected function name');
}
const args = properties.args.map((v: any) => this.Operand(v));
return [sqlName ?? (_.capitalize(name) as Capitalize<T>), ...args];
return [sqlName ?? (capitalize(name) as Capitalize<T>), ...args];
}
Operand(
match: any,
Expand Down Expand Up @@ -1558,7 +1566,7 @@ export class OData2AbstractSQL {
DateMatch(match: any, optional: true): StrictDateTypeNodes | undefined;
DateMatch(match: any): StrictDateTypeNodes;
DateMatch(match: any, optional = false): StrictDateTypeNodes | undefined {
if (_.isDate(match)) {
if (match instanceof Date) {
return ['Date', match];
} else if (Array.isArray(match) && match[0] === 'call') {
const { method } = match[1];
Expand Down Expand Up @@ -1592,11 +1600,17 @@ export class OData2AbstractSQL {
if (match == null || typeof match !== 'object') {
return;
}
const duration = _(match)
.pick('negative', 'day', 'hour', 'minute', 'second')
.omitBy(_.isNil)
.value();
if (_(duration).omit('negative').isEmpty()) {
const picked = pick(match, ['negative', 'day', 'hour', 'minute', 'second']);

const duration = omitBy(picked, isNil);

// Destructure 'negative' out, collect the 'rest' into a new object
Copy link
Contributor

@Page- Page- Dec 30, 2025

Choose a reason for hiding this comment

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

Again this comment is also kind of redundant as it only tells me what the code already tells me

const { negative, ...rest } = duration;

// Check if 'rest' is empty
Copy link
Contributor

@Page- Page- Dec 30, 2025

Choose a reason for hiding this comment

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

This comment is kind of redundant, it tells me what the code is doing but I can see that by simply reading the code and it doesn't add anything - if it were to say why then that could be useful but its current form is just redundant

const isEmptyDuration = Object.keys(rest).length === 0;

if (isEmptyDuration) {
Comment on lines +1611 to +1613
Copy link
Contributor

@Page- Page- Dec 30, 2025

Choose a reason for hiding this comment

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

There's no need to assign to an intermediary here:

Suggested change
const isEmptyDuration = Object.keys(rest).length === 0;
if (isEmptyDuration) {
if (Object.keys(rest).length === 0) {

return;
}
return ['Duration', duration];
Expand Down Expand Up @@ -1786,7 +1800,7 @@ export class OData2AbstractSQL {
});
if (existingJoin != null) {
const existingJoinPredicate = existingJoin[2]?.[1];
if (!_.isEqual(navigation.where, existingJoinPredicate)) {
if (!isEqual(navigation.where, existingJoinPredicate)) {
// When we reach this point we have found an already existing JOIN with the
// same alias as the one we just created but different ON predicate.
// TODO: In this case we need to be able to generate a new alias for the newly JOINed resource.
Expand Down Expand Up @@ -1837,7 +1851,7 @@ export class OData2AbstractSQL {
this.defaultResource = undefined;
}
Synonym(sqlName: string) {
return _(sqlName)
return sqlName
.split('-')
.map((namePart) => {
const synonym = this.clientModel.synonyms[namePart];
Expand Down Expand Up @@ -1894,7 +1908,7 @@ export class OData2AbstractSQL {
extraBindVars: ODataBinds,
bindVarsLength: number,
): ModernDefinition {
const rewrittenDefinition = _.cloneDeep(
const rewrittenDefinition = structuredClone(
convertToModernDefinition(definition),
);
rewriteBinds(rewrittenDefinition, extraBindVars, bindVarsLength);
Expand Down Expand Up @@ -1953,12 +1967,12 @@ const addAliases = (
// We then traverse any non suffix nodes to make sure those parts get their short versions. This should only happen
// in the case of a '' suffix because it means that the other parts are supersets, eg `of`/`often` and must be added
// with a longer short alias, eg `of`/`oft`
_.forEach(node, (value, key) => {
for (const [key, value] of Object.entries(node)) {
if (key === '$suffix') {
return;
continue; // Skips this iteration
}
traverseNodes(str + key, value);
});
}
};

lowerCaseAliasParts.sort().forEach(buildTrie);
Expand Down Expand Up @@ -1988,69 +2002,80 @@ const getRelationships = (
const generateShortAliases = (clientModel: RequiredAbstractSqlModelSubset) => {
const shortAliases: Dictionary<string> = {};

const aliasParts = _(getRelationships(clientModel.relationships))
.union(Object.keys(clientModel.synonyms))
.reject((key) => key === '$')
.map((alias) => alias.toLowerCase())
.value();
// 1. Safely get your source arrays (using ?? [] to handle nulls like Lodash would)
const rels = getRelationships(clientModel.relationships) ?? [];
Copy link
Contributor

@Page- Page- Dec 30, 2025

Choose a reason for hiding this comment

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

This always returns an array already so the ?? [] is completely pointless? It looks to me like this was written by an LLM and it's added a bunch of unnecessary parts

const synonyms = Object.keys(clientModel.synonyms ?? {});

// 2. Create the Union (Spread into a Set to remove duplicates, then back to Array)
const uniqueKeys = Array.from(new Set([...rels, ...synonyms]));

// 3. Filter and Map
const aliasParts = uniqueKeys
.filter((key) => key !== '$') // Native replacement for .reject()
.map((alias) => alias.toLowerCase());

// Add the first level of aliases, of names split by `-`/` `/`$`, for short aliases on a word by word basis
let origAliasParts = _(aliasParts)
.flatMap((aliasPart) => aliasPart.split(/-| |\$/))
.uniq()
.value();
let origAliasParts = Array.from(
new Set(aliasParts.flatMap((aliasPart) => aliasPart.split(/-| |\$/))),
);
addAliases(shortAliases, origAliasParts);

// Add aliases for $ containing names
origAliasParts = _(aliasParts)
.flatMap((aliasPart) => aliasPart.split(/-| /))
.filter((aliasPart) => aliasPart.includes('$'))
.flatMap((aliasPart) => {
shortAliases[aliasPart] = aliasPart
.split('$')
.map((part) => shortAliases[part])
.join('$');
return [];
})
.uniq()
.value();
origAliasParts = Array.from(
new Set(
aliasParts
.flatMap((aliasPart) => aliasPart.split(/-| /))
.filter((aliasPart) => aliasPart.includes('$'))
.flatMap((aliasPart) => {
shortAliases[aliasPart] = aliasPart
.split('$')
.map((part) => shortAliases[part])
.join('$');
return [];
}),
),
);
addAliases(shortAliases, origAliasParts);

// Add the second level of aliases, of names that include a ` `, split by `-`, for short aliases on a verb/term basis
origAliasParts = _(aliasParts)
.flatMap((aliasPart) => aliasPart.split('-'))
.filter((aliasPart) => aliasPart.includes(' '))
.flatMap((aliasPart) =>
// Generate the 2nd level aliases for both the short and long versions
[
aliasPart,
aliasPart
.split(' ')
.map((part) => shortAliases[part])
.join(' '),
],
)
.uniq()
.value();
origAliasParts = Array.from(
new Set(
aliasParts
.flatMap((aliasPart) => aliasPart.split('-'))
.filter((aliasPart) => aliasPart.includes(' '))
.flatMap((aliasPart) =>
// Generate the 2nd level aliases for both the short and long versions
[
aliasPart,
aliasPart
.split(' ')
.map((part) => shortAliases[part])
.join(' '),
],
),
),
);

addAliases(shortAliases, origAliasParts);

// Add the third level of aliases, of names that include a `-`, for short aliases on a fact type basis
origAliasParts = _(aliasParts)
.filter((aliasPart) => aliasPart.includes('-'))
.flatMap((aliasPart) =>
// Generate the 3rd level aliases for both the short and long versions

[
aliasPart,
aliasPart
.split('-')
.map((part) => shortAliases[part])
.join('-'),
],
)
.uniq()
.value();
origAliasParts = Array.from(
new Set(
aliasParts
.filter((aliasPart) => aliasPart.includes('-'))
.flatMap((aliasPart) =>
// Generate the 3rd level aliases for both the short and long versions

[
aliasPart,
aliasPart
.split('-')
.map((part) => shortAliases[part])
.join('-'),
],
),
),
);

addAliases(shortAliases, origAliasParts);

Expand Down
14 changes: 7 additions & 7 deletions test/chai-sql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import _ from 'lodash';
import { get } from 'es-toolkit/compat';
import * as chai from 'chai';
import chaiThings from 'chai-things';
import fs from 'node:fs';
Expand Down Expand Up @@ -205,7 +205,7 @@ export function operandToAbstractSQLFactory(
binds.push(['Number', operand]);
return ['Bind', binds.length - 1];
}
if (_.isDate(operand)) {
if (operand instanceof Date) {
binds.push(['Date', operand]);
return ['Bind', binds.length - 1];
}
Expand Down Expand Up @@ -238,11 +238,11 @@ export function operandToAbstractSQLFactory(
const fieldParts = operand.split('/');
if (fieldParts.length > 1) {
let alias = parentAlias;
let previousResource = _(parentAlias).split('.').last()!;
let previousResource = parentAlias.split('.').at(-1)!;
for (const resourceName of fieldParts.slice(0, -1)) {
const sqlName = odataNameToSqlName(resourceName);
const sqlNameParts = sqlName.split('-');
mapping = _.get(
mapping = get(
clientModel.relationships[previousResource],
sqlNameParts,
).$;
Expand All @@ -255,7 +255,7 @@ export function operandToAbstractSQLFactory(
}
previousResource = refTable;
}
mapping = [alias, _.last(fieldParts)];
mapping = [alias, fieldParts.at(-1)];
} else {
mapping = [resource, odataNameToSqlName(operand)];
}
Expand Down Expand Up @@ -289,7 +289,7 @@ export const operandToOData = function (operand): string {
if (operand.odata != null) {
return operand.odata;
}
if (_.isDate(operand)) {
if (operand instanceof Date) {
return "datetime'" + encodeURIComponent(operand.toISOString()) + "'";
}
if (Array.isArray(operand)) {
Expand Down Expand Up @@ -369,7 +369,7 @@ export const aliasFields = (function () {
} else {
verb = '';
}
return fields.map(_.partial(aliasField, resourceAlias, verb));
return fields.map((field) => aliasField(resourceAlias, verb, field));
};
})();

Expand Down
Loading
Loading