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

Handle explicit undefined values #8

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

Conversation

xurion
Copy link

@xurion xurion commented Dec 23, 2022

In JSON, you can explicitly define a value as undefined:

{ _undefined: undefined }

When running the format function against the above object, the following error is observed:

/Users/dean/projects/lua-json/index.js:53
    throw new Error(`can't format ${typeof value}`)
    ^

Error: can't format undefined
    at rec (/Users/dean/projects/lua-json/index.js:53:11)
    at /Users/dean/projects/lua-json/index.js:46:79
    at Array.map (<anonymous>)
    at rec (/Users/dean/projects/lua-json/index.js:46:12)
    at format (/Users/dean/projects/lua-json/index.js:56:47)
    at Object.<anonymous> (/Users/dean/projects/lua-json/test.js:70:7)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)

This commit adds the missing safeguard against any explicit undefined values which prevents the fall through to throw new Error('can't format ${typeof value}'). Adding this surfaces a small issue wherein \n\n is written to the formatted string. As far as I can tell, this is due to the way the ${eol} is prepended (rather than appended) to each line. There is a .replace method call to take care of the \n\n, however, this has a limit. For example, \n\n\n would still become \n\n.

Even accounting for the above double-line issue, this does fix the fatal error when dealing with undefined values and still produces a valid Lua string.

Edit: I solved the above with a simple do...while loop.

A specific test case was added because the shape of the JSON and Lua are not interchangeable. For example:

format({ _undefined: undefined, _string: 'string' }) // -> 'return { _string = "string" }'
parse('return { _string = "string" }') // -> { _string: 'string' }

@gakada
Copy link
Collaborator

gakada commented Dec 28, 2022

JSON itself has no undefined, though JSON.stringify does quite a bit of cleaning when converting JavaScript values to JSON strings. I think by default the library should fail on such cases, so that parse(format(x)) ~ x is guaranteed (if otherwise possible), and then there can be optional cleaning, e.g. by doing value = JSON.parse(JSON.stringify(value)) first in format, or maybe that should be left to the client code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants