fix: properly serialize undefined in vuex store #3913

aldarund · 2018-09-18T20:08:27Z

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Description

Resolves #3825
serialize-javascript dont serialize undefined values.
yahoo/serialize-javascript#35
This PR use https://github.com/Rich-Harris/devalue for json serialization instead. JSON.stringify not used to avoid XSS

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly. (PR: #)
I have added tests to cover my changes (if not applicable, please state why)
All new and existing tests passed.

codecov-io · 2018-09-18T21:09:20Z

Codecov Report

Merging #3913 into dev will decrease coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              dev    #3913      +/-   ##
==========================================
- Coverage   97.56%   97.48%   -0.09%     
==========================================
  Files          18       18              
  Lines        1232     1232              
  Branches      337      337              
==========================================
- Hits         1202     1201       -1     
- Misses         29       30       +1     
  Partials        1        1

Impacted Files	Coverage Δ
lib/builder/builder.js	`96.31% <ø> (ø)`	⬆️
lib/core/renderer.js	`100% <100%> (ø)`	⬆️
lib/core/nuxt.js	`94.16% <0%> (-0.84%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55a153c...c0a5ed4. Read the comment docs.

manniL · 2018-09-20T07:31:03Z

Thanks for you effort @aldarund ☺️

If devalue provides more value (no pun intended) and is a drop-in replacement for serialize-javascript I'd advocate to fully replace serialize-javascript with it.

No inconsistencies
Better support of common structures
- Map
- Set
- undefined
- cyclical references,
- repeated references

manniL · 2018-09-20T07:39:56Z

We should also keep in mind that devalue does not support function serialization and has no plan to add this.

aldarund · 2018-09-20T09:12:48Z

@manniL i did replaced serialize-javascript everywhere where it was used for json. As i see in other places it is used for javascript serialization ( at least calls to serialize-javascript dont have isJson argument) so it probably cant be replaced entirely

Atinux · 2018-09-24T15:46:36Z

Could you fix conflicts @aldarund ?

aldarund · 2018-09-24T16:02:16Z

conflicts fixed

# Conflicts: # package.json # yarn.lock

pi0 · 2018-09-25T08:32:38Z

Waiting to CI passing :)

appinteractive · 2018-09-26T15:58:33Z

Hey the new option kills all methods (like formatters for tables) when serializing. That’s intended right?

aldarund · 2018-09-26T16:14:13Z

@appinteractive not quite follow. What u do mean by methods? Functions? If functions they were killed before as well
See here
https://runkit.com/5baa3cac92a46f00121e2985/5babafa1191da800121dc83f

benfavre · 2018-10-01T12:02:30Z

Getting isues with this now.

✖ error NuxtServerError: Cannot stringify a function
at walk (xxx/node_modules/devalue/dist/devalue.umd.js:16:19)
at xxx/node_modules/devalue/dist/devalue.umd.js:49:72
at Array.forEach ()
at walk (xxx/node_modules/devalue/dist/devalue.umd.js:49:40)
at xxx/node_modules/devalue/dist/devalue.umd.js:49:72
at Array.forEach ()
at walk (xxx/node_modules/devalue/dist/devalue.umd.js:49:40)
at xxx/node_modules/devalue/dist/devalue.umd.js:49:72
at Array.forEach ()
at walk (xxx/node_modules/devalue/dist/devalue.umd.js:49:40)
at devalue (xxx/node_modules/devalue/dist/devalue.umd.js:53:5)
at Renderer.renderRoute (xxx/node_modules/nuxt/dist/nuxt.js:2042:50)
at process._tickCallback (internal/process/next_tick.js:68:7)

manniL · 2018-10-01T12:07:53Z

@benfavre can you provide us your state? Likely there is a function inside

benfavre · 2018-10-01T12:16:29Z

benfavre · 2018-10-01T12:20:06Z

apparently we have some new i18n issues now 💃
Stupid fix for now :

manniL · 2018-10-01T12:32:49Z

@benfavre Could you open up an issue here ?

manniL · 2018-10-01T12:35:56Z

@benfavre According to the source code messages should never be a function, only an object 🤔

benfavre · 2018-10-01T13:54:54Z

This is because I use the Lazy option.
https://nuxt-community.github.io/nuxt-i18n/lazy-load-translations.html

manniL · 2018-10-01T14:29:38Z

@benfavre But even with lazy it should await the promise properly (source)

benfavre · 2018-10-01T14:55:41Z

apparently not, nothing special in my lang files

manniL · 2018-10-01T15:03:43Z

Weird. Could you open up a new issue with reproduction as request (here)? Let's continue the discussion there.

benfavre · 2018-10-01T15:59:30Z

https://cmty.app forces a repro link ... randomly pasted jsfiddle (probably going to get many of theses)

vhf · 2018-10-03T20:56:25Z

I think this should have been a breaking change, I rolled back to nuxt<2.1.0 because my project throws Cannot stringify arbitrary non-POJOs from devalue now. ;)

(Just a heads up, I'm not complaining, thanks for the awesome work!)

Repro is basically this: https://github.com/Rich-Harris/devalue/blob/d73708c51f70bf9b962103976b44e782b1e3a068/test/test.ts#L89-L93 , putting foo in nuxt's vuex store.

I know what I'm doing is weird and I would totally understand if nuxt doesn't want to support that kind of hacks. I'm storing stuff that I only need server-side and stuff that I only need client-side. These are complex values and it works well as long as I put them in the store and retrieve them on the same side. Before devalue was added to this project nuxt was simply deserializing into something unusable (as expected since state is passed between client/server as JSON, therefore dropping prototype infos, methods on objects, etc) but I didn't care anyway as long as it wasn't crashing the server (or even showing a warning for that matter), which is now the case. 😄

manniL · 2018-10-03T21:41:27Z

@vhf We are working on replacing the error with "just" a warning to revert the breaking change.

lock · 2018-11-02T22:15:02Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

aldarund added 3 commits September 18, 2018 22:43

allow to serialize undefined variable in vuex store

a441f7b

test properly

64f3f30

fix test

739a39d

aldarund changed the title ~~Properly serialize undefined in vuex store~~ fix: properly serialize undefined in vuex store Sep 19, 2018

galvez assigned aldarund Sep 20, 2018

galvez requested review from clarkdo, pi0, Atinux and manniL September 20, 2018 00:07

galvez added post-2.0 and removed post-2.0 labels Sep 20, 2018

aldarund added 2 commits September 24, 2018 19:03

Merge branch 'dev' into store-undef-var

15f1ac5

# Conflicts: # package.json # yarn.lock

lock

c201c2c

manniL approved these changes Sep 24, 2018

View reviewed changes

galvez approved these changes Sep 24, 2018

View reviewed changes

pi0 approved these changes Sep 25, 2018

View reviewed changes

Merge branch 'dev' into store-undef-var

c0a5ed4

Atinux merged commit df148a8 into nuxt:dev Sep 25, 2018

aldarund mentioned this pull request Sep 25, 2018

fix: add missing dep for legacy and start #3963

Merged

7 tasks

pimlie mentioned this pull request Oct 1, 2018

NuxtServerError: Cannot stringify a function #4025

Closed

benfavre mentioned this pull request Oct 3, 2018

error NuxtServerError: Cannot stringify a function #4026

Closed

lock bot locked as resolved and limited conversation to collaborators Nov 2, 2018

danielroe added the 2.x label Jan 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: properly serialize undefined in vuex store #3913

fix: properly serialize undefined in vuex store #3913

aldarund commented Sep 18, 2018 •

edited

codecov-io commented Sep 18, 2018 •

edited

manniL commented Sep 20, 2018

manniL commented Sep 20, 2018

aldarund commented Sep 20, 2018 •

edited

Atinux commented Sep 24, 2018

aldarund commented Sep 24, 2018

pi0 commented Sep 25, 2018

appinteractive commented Sep 26, 2018

aldarund commented Sep 26, 2018

benfavre commented Oct 1, 2018

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018

benfavre commented Oct 1, 2018 •

edited

manniL commented Oct 1, 2018 •

edited

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018 •

edited

vhf commented Oct 3, 2018 •

edited

manniL commented Oct 3, 2018

lock bot commented Nov 2, 2018

fix: properly serialize undefined in vuex store #3913

fix: properly serialize undefined in vuex store #3913

Conversation

aldarund commented Sep 18, 2018 • edited

Types of changes

Description

Checklist:

codecov-io commented Sep 18, 2018 • edited

Codecov Report

manniL commented Sep 20, 2018

manniL commented Sep 20, 2018

aldarund commented Sep 20, 2018 • edited

Atinux commented Sep 24, 2018

aldarund commented Sep 24, 2018

pi0 commented Sep 25, 2018

appinteractive commented Sep 26, 2018

aldarund commented Sep 26, 2018

benfavre commented Oct 1, 2018

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018

benfavre commented Oct 1, 2018 • edited

manniL commented Oct 1, 2018 • edited

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018

manniL commented Oct 1, 2018

benfavre commented Oct 1, 2018 • edited

vhf commented Oct 3, 2018 • edited

manniL commented Oct 3, 2018

lock bot commented Nov 2, 2018

aldarund commented Sep 18, 2018 •

edited

codecov-io commented Sep 18, 2018 •

edited

aldarund commented Sep 20, 2018 •

edited

benfavre commented Oct 1, 2018 •

edited

manniL commented Oct 1, 2018 •

edited

benfavre commented Oct 1, 2018 •

edited

vhf commented Oct 3, 2018 •

edited