Skip to content
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

perf: optimize required props serialization #675

Merged
merged 1 commit into from
Jan 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
114 changes: 50 additions & 64 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ const validLargeArrayMechanisms = [
'json-stringify'
]

const addComma = '!addComma && (addComma = true) || (json += \',\')'

let schemaIdCounter = 0

const mergedSchemaRef = Symbol('fjs-merged-schema-ref')
Expand Down Expand Up @@ -262,7 +260,7 @@ function inferTypeByKeyword (schema) {
return schema.type
}

function buildExtraObjectPropertiesSerializer (context, location) {
function buildExtraObjectPropertiesSerializer (context, location, addComma) {
const schema = location.schema
const propertiesKeys = Object.keys(schema.properties || {})

Expand Down Expand Up @@ -321,88 +319,76 @@ function buildExtraObjectPropertiesSerializer (context, location) {
}

function buildInnerObject (context, location) {
let code = ''
const schema = location.schema
const required = schema.required || []

const propertiesLocation = location.getPropertyLocation('properties')
const requiredProperties = schema.required || []

// Should serialize required properties first
const propertiesKeys = Object.keys(schema.properties || {}).sort(
(key1, key2) => {
const required1 = requiredProperties.includes(key1)
const required2 = requiredProperties.includes(key2)
return required1 === required2 ? 0 : required1 ? -1 : 1
}
)
const hasRequiredProperties = requiredProperties.includes(propertiesKeys[0])

const requiredWithDefault = []
const requiredWithoutDefault = []
if (schema.properties) {
for (const key of Object.keys(schema.properties)) {
if (required.indexOf(key) === -1) {
continue
}
let propertyLocation = propertiesLocation.getPropertyLocation(key)
if (propertyLocation.schema.$ref) {
propertyLocation = resolveRef(context, propertyLocation)
}

const sanitizedKey = JSON.stringify(key)
let code = ''

// Using obj['key'] !== undefined instead of obj.hasOwnProperty(prop) for perf reasons,
// see https://github.com/mcollina/fast-json-stringify/pull/3 for discussion.
const defaultValue = propertyLocation.schema.default
if (defaultValue === undefined) {
code += `if (obj[${sanitizedKey}] === undefined) throw new Error('${sanitizedKey} is required!')\n`
requiredWithoutDefault.push(key)
}
requiredWithDefault.push(key)
for (const key of requiredProperties) {
if (!propertiesKeys.includes(key)) {
code += `if (obj['${key}'] === undefined) throw new Error('"${key}" is required!')\n`
}
}

// handle extraneous required fields
for (const requiredProperty of required) {
if (requiredWithDefault.indexOf(requiredProperty) !== -1) continue
code += `if (obj['${requiredProperty}'] === undefined) throw new Error('"${requiredProperty}" is required!')\n`
}
code += 'let json = \'{\'\n'

code += `
let addComma = false
let json = '{'
`
let addComma = ''
if (!hasRequiredProperties) {
code += 'let addComma = false\n'
addComma = '!addComma && (addComma = true) || (json += \',\')'
}

if (schema.properties) {
for (const key of Object.keys(schema.properties)) {
let propertyLocation = propertiesLocation.getPropertyLocation(key)
if (propertyLocation.schema.$ref) {
propertyLocation = resolveRef(context, propertyLocation)
}
for (const key of propertiesKeys) {
let propertyLocation = propertiesLocation.getPropertyLocation(key)
if (propertyLocation.schema.$ref) {
propertyLocation = resolveRef(context, propertyLocation)
}

const sanitizedKey = JSON.stringify(key)
const sanitizedKey = JSON.stringify(key)
const defaultValue = propertyLocation.schema.default
const isRequired = requiredProperties.includes(key)

if (requiredWithoutDefault.indexOf(key) !== -1) {
code += `
code += `
if (obj[${sanitizedKey}] !== undefined) {
${addComma}
json += ${JSON.stringify(sanitizedKey + ':')}
${buildValue(context, propertyLocation, `obj[${sanitizedKey}]`)}
}`

if (defaultValue !== undefined) {
code += ` else {
${addComma}
json += ${JSON.stringify(sanitizedKey + ':' + JSON.stringify(defaultValue))}
}
`
} else {
// Using obj['key'] !== undefined instead of obj.hasOwnProperty(prop) for perf reasons,
// see https://github.com/mcollina/fast-json-stringify/pull/3 for discussion.
code += `
if (obj[${sanitizedKey}] !== undefined) {
${addComma}
json += ${JSON.stringify(sanitizedKey + ':')}
${buildValue(context, propertyLocation, `obj[${sanitizedKey}]`)}
}
`
const defaultValue = propertyLocation.schema.default
if (defaultValue !== undefined) {
code += `
else {
${addComma}
json += ${JSON.stringify(sanitizedKey + ':' + JSON.stringify(defaultValue))}
}
`
}
} else if (isRequired) {
code += ` else {
throw new Error('${sanitizedKey} is required!')
}
`
} else {
code += '\n'
}

if (hasRequiredProperties) {
addComma = 'json += \',\''
}
}

if (schema.patternProperties || schema.additionalProperties) {
code += buildExtraObjectPropertiesSerializer(context, location)
code += buildExtraObjectPropertiesSerializer(context, location, addComma)
}

code += `
Expand Down
2 changes: 1 addition & 1 deletion test/allof.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ test('object with allOf and multiple schema on the allOf', (t) => {
id: 1,
name: 'string',
tag: 'otherString'
}), '{"name":"string","tag":"otherString","id":1}')
}), '{"name":"string","id":1,"tag":"otherString"}')
})

test('object with allOf and one schema on the allOf', (t) => {
Expand Down
7 changes: 4 additions & 3 deletions test/required.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,18 @@ test('object with multiple required field not in properties schema', (t) => {
stringify({})
t.fail()
} catch (e) {
t.equal(e.message, '"num" is required!')
t.equal(e.message, '"key1" is required!')
t.pass()
}

try {
stringify({
num: 42
key1: 42,
key2: 42
})
t.fail()
} catch (e) {
t.equal(e.message, '"key1" is required!')
t.equal(e.message, '"num" is required!')
t.pass()
}

Expand Down
Loading