Skip to content

Commit 8932bcf

Browse files
committed
This set of changes allows JSON Logic engine to more strictly honor sugaring in most methods.
While the operators I implemented were compatible with the base spec, the interpreter was not implemented to actually sugar / desugar single arguments. In terms of compatibility, this has not created issues for anyone, but it was possible to add new operators that would not receive arrays as arguments. I considered this to be a good thing / an option to be flexible, but with https://github.com/json-logic, I'd like to try to be more rigid with what I allow. However, I've compromised by allowing users to add a flag to enable them to say `optimizeUnary` on an operator. This argument is used to signify "Hey, I support arrays, but I also support direct input if you want to invoke me with that". This allows my compiler & optimizer to avoid array destructuring overhead, which can actually have semi-significant impact (it did in the Handlebars JLE Library) Because this change makes adding operators a bit more rigid, and semi-ensures that args will always be an array, I'm bumping the major version flag once more. I've also changed the layout for some of my tests. Over time, I'm going to move more JSON files into the `suites` directory.
1 parent 42b1643 commit 8932bcf

18 files changed

+147
-46
lines changed

asLogic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ function pick (keep, obj) {
2121
export function asLogicSync (functions, keep = ['var'], engine = new LogicEngine()) {
2222
engine.methods = pick(keep, engine.methods)
2323
engine.addMethod('list', i => [].concat(i))
24-
Object.keys(functions).forEach(i => engine.addMethod(i, data => Array.isArray(data) ? functions[i](...data) : functions[i](data === null ? undefined : data)))
24+
Object.keys(functions).forEach(i => engine.addMethod(i, data => functions[i](...data)))
2525
return engine.build.bind(engine)
2626
}
2727

asLogic.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { asLogicSync, asLogicAsync } from './asLogic.js'
22

33
const module = {
4-
hello: (name = 'World', last = '') => `Hello, ${name}${last.length ? ' ' : ''}${last}!`
4+
hello: (name = 'World', last = '') => `Hello, ${name ?? 'World'}${last.length ? ' ' : ''}${last}!`
55
}
66

77
describe('asLogicSync', () => {

async.test.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const modes = [
66
]
77

88
for (const engine of modes) {
9-
engine.addMethod('as1', async (n) => n + 1, { async: true, deterministic: true })
9+
engine.addMethod('as1', async ([n]) => n + 1, { async: true, deterministic: true })
1010
}
1111

1212
modes.forEach((logic) => {
@@ -676,7 +676,7 @@ modes.forEach((logic) => {
676676
length: ['hello']
677677
})
678678

679-
expect(answer).toStrictEqual(1)
679+
expect(answer).toStrictEqual(5)
680680
})
681681

682682
test('length object (2 keys)', async () => {
@@ -781,7 +781,7 @@ modes.forEach((logic) => {
781781

782782
describe('addMethod', () => {
783783
test('adding a method works', async () => {
784-
logic.addMethod('+1', (item) => item + 1, { sync: true })
784+
logic.addMethod('+1', ([item]) => item + 1, { sync: true })
785785
expect(
786786
await logic.run({
787787
'+1': 7

asyncLogic.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { buildAsync } from './compiler.js'
99
import omitUndefined from './utilities/omitUndefined.js'
1010
import { optimize } from './async_optimizer.js'
1111
import { applyPatches } from './compatibility.js'
12+
import { coerceArray } from './utilities/coerceArray.js'
1213

1314
/**
1415
* An engine capable of running asynchronous JSON Logic.
@@ -75,16 +76,15 @@ class AsyncLogicEngine {
7576
if (!this.methods[func]) throw new Error(`Method '${func}' was not found in the Logic Engine.`)
7677

7778
if (typeof this.methods[func] === 'function') {
78-
const input = await this.run(data, context, { above })
79-
const result = await this.methods[func](input, context, above, this)
79+
const input = (!data || typeof data !== 'object') ? [data] : await this.run(data, context, { above })
80+
const result = await this.methods[func](coerceArray(input), context, above, this)
8081
return Array.isArray(result) ? Promise.all(result) : result
8182
}
8283

8384
if (typeof this.methods[func] === 'object') {
8485
const { asyncMethod, method, traverse } = this.methods[func]
8586
const shouldTraverse = typeof traverse === 'undefined' ? true : traverse
86-
const parsedData = shouldTraverse ? await this.run(data, context, { above }) : data
87-
87+
const parsedData = shouldTraverse ? ((!data || typeof data !== 'object') ? [data] : coerceArray(await this.run(data, context, { above }))) : data
8888
const result = await (asyncMethod || method)(parsedData, context, above, this)
8989
return Array.isArray(result) ? Promise.all(result) : result
9090
}
@@ -96,12 +96,12 @@ class AsyncLogicEngine {
9696
*
9797
* @param {String} name The name of the method being added.
9898
* @param {((args: any, context: any, above: any[], engine: AsyncLogicEngine) => any) | { traverse?: Boolean, method?: (args: any, context: any, above: any[], engine: AsyncLogicEngine) => any, asyncMethod?: (args: any, context: any, above: any[], engine: AsyncLogicEngine) => Promise<any>, deterministic?: Function | Boolean }} method
99-
* @param {{ deterministic?: Boolean, async?: Boolean, sync?: Boolean }} annotations This is used by the compiler to help determine if it can optimize the function being generated.
99+
* @param {{ deterministic?: Boolean, async?: Boolean, sync?: Boolean, optimizeUnary?: boolean }} annotations This is used by the compiler to help determine if it can optimize the function being generated.
100100
*/
101101
addMethod (
102102
name,
103103
method,
104-
{ deterministic, async, sync } = {}
104+
{ deterministic, async, sync, optimizeUnary } = {}
105105
) {
106106
if (typeof async === 'undefined' && typeof sync === 'undefined') sync = false
107107
if (typeof sync !== 'undefined') async = !sync
@@ -112,7 +112,7 @@ class AsyncLogicEngine {
112112
else method = { method, traverse: true }
113113
} else method = { ...method }
114114

115-
Object.assign(method, omitUndefined({ deterministic }))
115+
Object.assign(method, omitUndefined({ deterministic, optimizeUnary }))
116116
// @ts-ignore
117117
this.fallback.addMethod(name, method, { deterministic })
118118
this.methods[name] = declareSync(method, sync)
@@ -188,6 +188,8 @@ class AsyncLogicEngine {
188188
async build (logic, options = {}) {
189189
const { above = [], top = true } = options
190190
this.fallback.truthy = this.truthy
191+
// @ts-ignore
192+
this.fallback.allowFunctions = this.allowFunctions
191193
if (top) {
192194
const constructedFunction = await buildAsync(logic, { engine: this, above, async: true, state: {} })
193195

async_optimizer.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { isDeterministic } from './compiler.js'
33
import { map } from './async_iterators.js'
44
import { isSync, Sync } from './constants.js'
55
import declareSync from './utilities/declareSync.js'
6+
import { coerceArray } from './utilities/coerceArray.js'
67

78
/**
89
* Turns an expression like { '+': [1, 2] } into a function that can be called with data.
@@ -26,7 +27,8 @@ function getMethod (logic, engine, methodName, above) {
2627
return (data, abv) => called(args, data, abv || above, engine)
2728
}
2829

29-
const args = logic[methodName]
30+
let args = logic[methodName]
31+
if (!args || typeof args !== 'object') args = [args]
3032

3133
if (Array.isArray(args)) {
3234
const optimizedArgs = args.map(l => optimize(l, engine, above))
@@ -48,11 +50,11 @@ function getMethod (logic, engine, methodName, above) {
4850

4951
if (isSync(optimizedArgs) && (method.method || method[Sync])) {
5052
const called = method.method ? method.method : method
51-
return declareSync((data, abv) => called(typeof optimizedArgs === 'function' ? optimizedArgs(data, abv) : optimizedArgs, data, abv || above, engine), true)
53+
return declareSync((data, abv) => called(coerceArray(typeof optimizedArgs === 'function' ? optimizedArgs(data, abv) : optimizedArgs, method.optimizeUnary), data, abv || above, engine), true)
5254
}
5355

5456
return async (data, abv) => {
55-
return called(typeof optimizedArgs === 'function' ? await optimizedArgs(data, abv) : optimizedArgs, data, abv || above, engine)
57+
return called(coerceArray(typeof optimizedArgs === 'function' ? await optimizedArgs(data, abv) : optimizedArgs, method.optimizeUnary), data, abv || above, engine)
5658
}
5759
}
5860
}
@@ -65,6 +67,7 @@ function getMethod (logic, engine, methodName, above) {
6567
* @returns A function that optimizes the logic for the engine in advance.
6668
*/
6769
export function optimize (logic, engine, above = []) {
70+
engine.fallback.allowFunctions = engine.allowFunctions
6871
if (Array.isArray(logic)) {
6972
const arr = logic.map(l => optimize(l, engine, above))
7073
if (isSync(arr)) return declareSync((data, abv) => arr.map(l => typeof l === 'function' ? l(data, abv) : l), true)

bench/incompatible.json

Lines changed: 0 additions & 1 deletion
This file was deleted.

build.test.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,11 @@ function timeout (n, x) {
231231
expect(await f({ x: 1, y: 2 })).toStrictEqual({ a: 1, b: 2 })
232232
})
233233

234-
test('Invalid eachKey', async () => {
235-
expect(async () => await logic.build({ eachKey: 5 })).rejects.toThrow(
236-
InvalidControlInput
237-
)
238-
})
234+
// test('Invalid eachKey', async () => {
235+
// expect(async () => await logic.build({ eachKey: 5 })).rejects.toThrow(
236+
// InvalidControlInput
237+
// )
238+
// })
239239

240240
test('Simple deterministic eachKey', async () => {
241241
const f = await logic.build({ eachKey: { a: 1, b: { '+': [1, 1] } } })
@@ -246,7 +246,7 @@ function timeout (n, x) {
246246
})
247247

248248
const logic = new AsyncLogicEngine()
249-
logic.addMethod('as1', async (n) => timeout(100, n + 1), { async: true })
249+
logic.addMethod('as1', async ([n]) => timeout(100, n + 1), { async: true })
250250

251251
describe('Testing async build with full async', () => {
252252
test('Async +1', async () => {

compatible.test.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import fs from 'fs'
22
import { LogicEngine, AsyncLogicEngine } from './index.js'
3-
const tests = JSON.parse(fs.readFileSync('./bench/compatible.json').toString())
3+
const tests = []
4+
5+
// get all json files from "suites" directory
6+
const files = fs.readdirSync('./suites')
7+
for (const file of files) {
8+
if (file.endsWith('.json')) tests.push(...JSON.parse(fs.readFileSync(`./suites/${file}`).toString()).filter(i => typeof i !== 'string'))
9+
}
410

511
// eslint-disable-next-line no-labels
612
inline: {

compiler.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import declareSync from './utilities/declareSync.js'
1010

1111
// asyncIterators is required for the compiler to operate as intended.
1212
import asyncIterators from './async_iterators.js'
13+
import { coerceArray } from './utilities/coerceArray.js'
1314

1415
/**
1516
* Provides a simple way to compile logic into a function that can be run.
@@ -191,25 +192,32 @@ function buildString (method, buildState = {}) {
191192
}
192193
}
193194

195+
let lower = method[func]
196+
if (!lower || typeof lower !== 'object') lower = [lower]
197+
194198
if (engine.methods[func] && engine.methods[func].compile) {
195-
let str = engine.methods[func].compile(method[func], buildState)
199+
let str = engine.methods[func].compile(lower, buildState)
196200
if (str[Compiled]) str = str[Compiled]
197201

198202
if ((str || '').startsWith('await')) buildState.asyncDetected = true
199203

200204
if (str !== false) return str
201205
}
202206

207+
let coerce = engine.methods[func].optimizeUnary ? '' : 'coerceArray'
208+
if (!coerce && Array.isArray(lower) && lower.length === 1) lower = lower[0]
209+
else if (coerce && Array.isArray(lower)) coerce = ''
210+
203211
if (typeof engine.methods[func] === 'function') {
204212
asyncDetected = !isSync(engine.methods[func])
205-
return makeAsync(`engine.methods["${func}"](` + buildString(method[func], buildState) + ', context, above, engine)')
213+
return makeAsync(`engine.methods["${func}"](${coerce}(` + buildString(lower, buildState) + '), context, above, engine)')
206214
} else {
207215
if (engine.methods[func] && (typeof engine.methods[func].traverse === 'undefined' ? true : engine.methods[func].traverse)) {
208216
asyncDetected = Boolean(async && engine.methods[func] && engine.methods[func].asyncMethod)
209-
return makeAsync(`engine.methods["${func}"]${asyncDetected ? '.asyncMethod' : '.method'}(` + buildString(method[func], buildState) + ', context, above, engine)')
217+
return makeAsync(`engine.methods["${func}"]${asyncDetected ? '.asyncMethod' : '.method'}(${coerce}(` + buildString(lower, buildState) + '), context, above, engine)')
210218
} else {
211219
asyncDetected = Boolean(async && engine.methods[func] && engine.methods[func].asyncMethod)
212-
notTraversed.push(method[func])
220+
notTraversed.push(lower)
213221
return makeAsync(`engine.methods["${func}"]${asyncDetected ? '.asyncMethod' : '.method'}(` + `notTraversed[${notTraversed.length - 1}]` + ', context, above, engine)')
214222
}
215223
}
@@ -294,12 +302,12 @@ function processBuiltString (method, str, buildState) {
294302
str = str.replace(`__%%%${x}%%%__`, item)
295303
})
296304

297-
const final = `(values, methods, notTraversed, asyncIterators, engine, above) => ${buildState.asyncDetected ? 'async' : ''} (context ${buildState.extraArguments ? ',' + buildState.extraArguments : ''}) => { const result = ${str}; return result }`
305+
const final = `(values, methods, notTraversed, asyncIterators, engine, above, coerceArray) => ${buildState.asyncDetected ? 'async' : ''} (context ${buildState.extraArguments ? ',' + buildState.extraArguments : ''}) => { const result = ${str}; return result }`
298306

299307
// console.log(str)
300308
// console.log(final)
301309
// eslint-disable-next-line no-eval
302-
return declareSync((typeof globalThis !== 'undefined' ? globalThis : global).eval(final)(values, methods, notTraversed, asyncIterators, engine, above), !buildState.asyncDetected)
310+
return declareSync((typeof globalThis !== 'undefined' ? globalThis : global).eval(final)(values, methods, notTraversed, asyncIterators, engine, above, coerceArray), !buildState.asyncDetected)
303311
}
304312

305313
export { build }

defaultMethods.js

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ const defaultMethods = {
178178
}
179179
return string.substr(from, end)
180180
},
181-
length: (i) => {
181+
length: ([i]) => {
182182
if (typeof i === 'string' || Array.isArray(i)) return i.length
183183
if (i && typeof i === 'object') return Object.keys(i).length
184184
return 0
@@ -398,7 +398,7 @@ const defaultMethods = {
398398
for (let i = 0; i < arr.length; i++) res += arr[i]
399399
return res
400400
},
401-
keys: (obj) => typeof obj === 'object' ? Object.keys(obj) : [],
401+
keys: ([obj]) => typeof obj === 'object' ? Object.keys(obj) : [],
402402
pipe: {
403403
traverse: false,
404404
[Sync]: (data, buildState) => isSyncDeep(data, buildState.engine, buildState),
@@ -799,13 +799,13 @@ defaultMethods.var.compile = function (data, buildState) {
799799
typeof data === 'number' ||
800800
(Array.isArray(data) && data.length <= 2)
801801
) {
802-
if (data === '../index' && buildState.iteratorCompile) return 'index'
803-
804802
if (Array.isArray(data)) {
805803
key = data[0]
806804
defaultValue = typeof data[1] === 'undefined' ? null : data[1]
807805
}
808806

807+
if (key === '../index' && buildState.iteratorCompile) return 'index'
808+
809809
// this counts the number of var accesses to determine if they're all just using this override.
810810
// this allows for a small optimization :)
811811
if (typeof key === 'undefined' || key === null || key === '') return 'context'
@@ -835,6 +835,9 @@ defaultMethods.var.compile = function (data, buildState) {
835835
return false
836836
}
837837

838+
// @ts-ignore Allowing a optimizeUnary attribute that can be used for performance optimizations
839+
defaultMethods['+'].optimizeUnary = defaultMethods['-'].optimizeUnary = defaultMethods.var.optimizeUnary = defaultMethods['!'].optimizeUnary = defaultMethods['!!'].optimizeUnary = defaultMethods.cat.optimizeUnary = true
840+
838841
export default {
839842
...defaultMethods
840843
}

0 commit comments

Comments
 (0)