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

fix(core): Modify safeStringify to allow multiple null values #4853

Merged
merged 1 commit into from Jul 20, 2021

Conversation

@thomasvidas
Copy link
Contributor

@thomasvidas thomasvidas commented Jul 20, 2021

Fixes #4798

@thomasvidas thomasvidas requested a review from elylucas Jul 20, 2021
@Ionitron Ionitron added this to Needs review 🤔 in Capacitor Engineering ⚡️ Jul 20, 2021
@thomasvidas
Copy link
Contributor Author

@thomasvidas thomasvidas commented Jul 20, 2021

Tested on this reproduction https://github.com/jepiqueau/capacitor-sqlite-test

Before fix
Screen Shot 2021-07-20 at 12 10 41 PM

After fix
Screen Shot 2021-07-20 at 12 10 03 PM

@thomasvidas thomasvidas merged commit 854539b into main Jul 20, 2021
17 checks passed
Capacitor Engineering ⚡️ automation moved this from Needs review 🤔 to Done 🎉 Jul 20, 2021
@thomasvidas thomasvidas deleted the null-safeStringify-fix branch Jul 20, 2021
@emirtb
Copy link

@emirtb emirtb commented Jul 20, 2021

Tested on this reproduction https://github.com/jepiqueau/capacitor-sqlite-test

Before fix
Screen Shot 2021-07-20 at 12 10 41 PM

After fix
Screen Shot 2021-07-20 at 12 10 03 PM

Is a null type or "null" string???

@thomasvidas
Copy link
Contributor Author

@thomasvidas thomasvidas commented Jul 21, 2021

It is a JSONObject with a value of null

@@ -200,7 +200,8 @@ const initBridge = (w: any): void => {
const seen = new Set();
return JSON.stringify(value, (_k, v) => {
if (seen.has(v)) {
return '...';
if (v === null) return null;

what about undefined values?

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

Successfully merging this pull request may close these issues.

4 participants