Skip to content

Skip undefined properties for options.json = true;#52

Open
ziir wants to merge 4 commits into
mathiasbynens:mainfrom
ziir:undefined-properties
Open

Skip undefined properties for options.json = true;#52
ziir wants to merge 4 commits into
mathiasbynens:mainfrom
ziir:undefined-properties

Conversation

@ziir

@ziir ziir commented Jun 27, 2019

Copy link
Copy Markdown

Hello, many thanks for this awesome package.

Similarily to JSON.stringify, jsesc(obj, { json:true }) should skip undefined properties within obj.

Side-note: Following discussions on Twitter, I was experimenting moving away from serialize-javascript to jsesc to serialize Redux SSR's initial state for GANDI apps such as https://shop.gandi.net when I encountered jsesc's behavior regarding undefined object properties, which differs from serialize-javascript's behavior.

Awaiting feedback before making final changes.

Fixes #17
Continuation of #18

Thank you for any response.

@mathiasbynens

Copy link
Copy Markdown
Owner

Please make the changes in /src/jsesc.js. /jsesc.js is generated from it when you run the build script.

@ziir ziir force-pushed the undefined-properties branch from c4cdf41 to 1613a09 Compare July 19, 2019 09:34
@ziir

ziir commented Jul 19, 2019

Copy link
Copy Markdown
Author

@mathiasbynens I have made the changes in the source, ran the build script and updated the README.
I'm not sure why the Travis build is failing though. Please advise.

Since recent changes, seems like Travis CI cannot run them within
25seconds
@ziir

ziir commented Jul 24, 2019

Copy link
Copy Markdown
Author

I just came across another case of different behavior between jsesc(..., { json: true }) (this PR's version) and JSON.stringify():

> require('jsesc')({ foo: () => {} }, { json: true })
'{"foo":null}'
> JSON.stringify({ foo: () => {} })
'{}'
> require('serialize-javascript')({ foo: () => {} }, { isJSON: true })
'{}'

I believe this falls in the scope of #17.

I will address it in the fork of jsesc I'm currently using for my project, and suggest a patch here.

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.

Document or remove differences between jsesc(data, { json: true }) and JSON.stringify(data)

2 participants