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

Double backslashes on double quotes in serialized Flux state? #174

Open
jedrichards opened this issue May 5, 2015 · 8 comments
Open

Double backslashes on double quotes in serialized Flux state? #174

jedrichards opened this issue May 5, 2015 · 8 comments

Comments

@jedrichards
Copy link

Hey

I have a simple store that serializes plain JS objects like so,

export default class ProjectStore extends Store {
  static serialize (state) {
    return JSON.stringify(state);
  }
  static deserialize (stateString) {
    return JSON.parse(stateString);
  }
  ...etc
}

And when I call my flux's serialize method server side I get a string somewhat like this (the escaped quotes being due to the fact that first I stringify the store and then you stringify the whole store tree, effectively double stringifying some parts of the output),

{"project":"{\"id\":\"7c5b02f7f4c80b2a5327aba1c3000c9a\",\"title\":\"Jolly trip\"}"}

I put this string into my app template (using Handlebars) and it ends up rendered on the page something like,

<script>
    var stateString = '{"project":"{\"id\":\"7c5b02f7f4c80b2a5327aba1c3000c9a\",\"title\":\"Jolly trip\"}"}'
</script>

The problem is that when I call flux.deserialize(stateString) in the client it chokes on it, specifically JSON.parse throws a wobbly. Afaiu this is because by the time stateString has been parsed into a string literal JS has striped out the backslashes (as expected), so that when I print it out in the console it looks like,

"{"project":"{"id":"7c5b02f7f4c80b2a5327aba1c3000c9a","title":"Jolly trip"}"}"

I don't really know what's going on there, aside from the fact that JSON.parse doesn't like it at all.

Now, I can fix this by doubling up on the backslashes server side before rendering the string into the template. Doing something like this,

let stateString = flux.serialize();
stateString = stateString.split('\\').join('\\\\');

Which results in rendered template,

<script>
  var stateString = '{"project":"{\\"id\\":\\"7c5b02f7f4c80b2a5327aba1c3000c9a\\",\\"title\\":\\"Jolly trip\\"}"}'
</script>

The above string feeds into flux.deserialize() quite happily and gets parsed properly and the app state recreated.

Just wondering if you've come across this sort of thing before? Have you found yourself double escaping backslashes, is it the right way to go or am I being thick?

Thanks!

Great package btw! :)

@jedrichards jedrichards changed the title Double escape backslashes on double quotes in serialized Flux state? Double backslashes on double quotes in serialized Flux state? May 6, 2015
@kjs3
Copy link

kjs3 commented May 21, 2015

@jedrichards I ran into this same issue and just stopped serializing/deserializing in my store's static methods so they look like this.

static serialize (state) {
  return state;
}

static deserialize (state) {
  return state;
}

Which gives me this warning.

The store with key 'search' was not serialized because the static method `SearchStore.serialize()` returned a non-string with type 'object'.

Which is untrue because I get back a JSON string and it all works great.
So, either I'm doing it wrong or that warning and the docs should change.

@jedrichards
Copy link
Author

Aha yes, nice one. That's simpler :)

Yeah, I think there's something a bit funny going on. Like you say, either the docs, warning, method naming or implementation could probably be changed to make things clearer.

I would expect to return properly serialized data from a method named serialize, not a plain object, but I wouldn't expect to have it stringified again by the library. Also, for anyone using immutable data structures in their store I think its an important step to handle that serialization explicitly.

@kjs3
Copy link

kjs3 commented May 21, 2015

Yeah, the double serializing is a real problem.

Right now serialize() at the higher level (in Flux.js) is just taking the output from the store's serialize() and assigning it to a key on a stateTree object. Then it's returning JSON.stringify(stateTree).

The outer JSON is so simple, what if we wrapped it explicitly without stringifying the whole thing?

@kjs3
Copy link

kjs3 commented May 21, 2015

Something like:

const serializedChunks = [];

Object.keys(stateTree).map(function(key){
  let wrappedValue = '"' + key + '":' + stateTree[key];
  serializedChunks.push(wrappedValue);
});

const finalOutput = '{' + serializedChunks.join(',') + '}';

@jedrichards
Copy link
Author

Yeah, that could work ...

Alternatively you could double up on backslashes in the ouput, something like,

return JSON.stringify(stateTree).split('\\').join('\\\\');

at https://github.com/acdlite/flummox/blob/master/src/Flux.js#L253

@johanneslumpe
Copy link
Collaborator

@jedrichards is this solved?

@jedrichards
Copy link
Author

I think this either needs solving in the library itself, or in the docs. As it is the serialisation API is quite confusing, and doesn't work as advertised. I'd be happy to put together a PR if people thought I was on the right track with the suggestion in my previous comment? Alternatively feel free to close this if you'd rather.

@kjs3
Copy link

kjs3 commented Jun 2, 2015

@jedrichards I just submitted a PR that removes the warning and adds some explanation to the docs. We'll see if that's a problem for anyone but that's how I'm using it in my project.

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

No branches or pull requests

4 participants