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

Support non-primitive values #171

Closed
wants to merge 1 commit into from

Conversation

Richienb
Copy link
Contributor

@Richienb Richienb commented Mar 4, 2021

Since this method replaces the normal serialization in order to work, we might want to store this behind an option instead. It might also make sense to move this to conf instead.

Fixes: #18


IssueHunt Summary

Signed-off-by: Richie Bendall <richiebendall@gmail.com>
@sindresorhus
Copy link
Owner

It should be implemented in Conf first, yes. I'm also not entirely sold on the current implementation as it makes the JSON store file just store a lot of Base64 binary items, which makes debugging really difficult.

@Richienb
Copy link
Contributor Author

What about serialize-javascript?

@sindresorhus
Copy link
Owner

Also see, sindresorhus/conf#89. I'm not entirely happy with that way either.

@sindresorhus
Copy link
Owner

What about serialize-javascript?

From a quick glance, it has a lot of issues. I think ideally, we would only use v8 for the types that are not natively supported by JSON. But we then need to also store in a meta-field which types should use native serialization and which should go through v8.

@sindresorhus
Copy link
Owner

But since we need to store it in a meta-field anyway, we could use a nicer human-friendly representation. Which brings us back to sindresorhus/conf#89.

@Richienb
Copy link
Contributor Author

Perhaps the serialiser can decide on what the meta-field should be called to avoid limitations on naming and serialise with v8.

The deserialiser can then look at the JSON and given the same set of rules the serialiser used, discover the name of the meta-field and reverse what it did.

@sindresorhus
Copy link
Owner

sindresorhus commented Mar 22, 2021

Perhaps the serialiser can decide on what the meta-field should be called to avoid limitations on naming and serialise with v8.

Not sure I understand what you mean. The meta-field name would be hard-coded. It would contain the same structure as the top-level ones. See the discussion in sindresorhus/conf#89.

@Cordelro
Copy link

Payment

@Cordelro
Copy link

#179

@Cordelro
Copy link

#189

@Cordelro
Copy link

#199

@Richienb
Copy link
Contributor Author

New plan: Make the replacer in JSON.stringify and the reviver in JSON.parse send non-primatives through the v8 serialize and deserialize functions so that they end up with a 2-part string as a value like this: __conf-${data}.

@Richienb
Copy link
Contributor Author

Continued in sindresorhus/conf#154

@Richienb Richienb closed this Sep 27, 2021
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.

Support dates
3 participants