-
Notifications
You must be signed in to change notification settings - Fork 30
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
jsurl v2 #17
base: master
Are you sure you want to change the base?
jsurl v2 #17
Conversation
Has nice watch mode and output diffs
I'd like to discuss the specs more before accepting a PR. Can we continue the discussion on #16? |
* prefix _ instead of - for special constants * () for objects * !~ for arrays
Very variable but gives an idea. v1 is twice as fast as v2 despite doing more work :( Needs profiling.
* not for final ~ representing `true` value * switch parser to using string indexes instead of splitting into array, so the end-of-object can be distinguished from ~
v2 parse is faster than v1, but stringify is slower
It stringifies slower than v1 and JSON, but it parses faster than v1 and JSON :)
rewrote stringifier to keep intermediate results in array and replaced join('~') with smart join performance.html:15 JSON: 200000 parsed in 731ms, 0.003655ms/item performance.html:23 JSON: 200000 stringified in 448ms, 0.00224ms/item performance.html:32 v1: 200000 parsed in 1337ms, 0.006685ms/item performance.html:40 v1: 200000 stringified in 934ms, 0.00467ms/item performance.html:49 v2: 200000 parsed in 601ms, 0.003005ms/item performance.html:57 v2: 200000 stringified in 403ms, 0.002015ms/item
So here's what I think is left to do:
Given that it is faster and smaller than JSON, I think I will use it to inject initial state for server-rendered SPA apps. To do so, it must be safe to put inside a |
Performance is impressive. Bravo! I did not have time yesterday but I'll review tonight. I just have a few points:
|
We can drop node 0.12 in unit tests BTW. |
|
I have mixed feelings about the optimization on I have another point: V1 was strictly aligned on JSON: For v1/v2 what about keeping the same package name but bumping the major version? Packages that depend on v1 will continue to get updates on v1. If your component only consumes feeds you can upgrade freely from 1.x to 2.x. but if it produces feeds consumed by other components you must check that the other components have been upgraded to 2.x. |
Not yet had time, but found this: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#toJSON()_behavior
|
No problem. I'm a bit overloaded too. I'm ok with the And yes, [off-topic] Saw that you are also using vscode. I got upgraded twice today 😃 . |
Alright:
A few non-code todos left (see top) [off-topic] Yes, vscode is awesome; the second release was because it ate my homework 😉 |
And now it imports v2 by default, v1 is at Added documentation. |
@wmertens. I still did not find the time. But I won't have any excuse this week-end. Sorry for that. |
No stress :)
In the mean time I also found
https://mathiasbynens.be/notes/javascript-escapes which means there should
be some more characters translated.
That plus making the sets of characters to target user definable needs to
be done still. (e.g. `stringify(obj, {target: 'js'})`) In my app, using
jsurl to stringify the initial redux state reduced the initial html by 30kb
(only 300 bytes difference after gzip, but still, that's 30kb that doesn't
have to be processed)
…On Fri, Apr 28, 2017 at 9:11 AM Bruno Jouhier ***@***.***> wrote:
@wmertens <https://github.com/wmertens>. I still did not find the time.
But I won't have any excuse this week-end. Sorry for that.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AADWlq6NIzg0VvqQHCQeVHyLaUonp_c2ks5r0ZEcgaJpZM4MwdpC>
.
|
If I understand correctly you also want an option to make it js-literal-friendly. Let me know when you are ready for merge. I'll do a detailed review then. |
Any progress on this? And will it handle replacing |
@yvele yes it doesn't use Comments welcome - I'm using it in production but I'm not storing anything long-term in it so I'm open to change encoding if necessary. |
Only exception: When `JSON.stringify()` returns `undefined`, JSURL returns `"_U"`
@bjouhier I did a bunch of cleaning up and making it more robust. Now it automatically parses JSON, v1 and v2, and it always outputs either valid JSON that decodes the same, or invalid JSON. Now it's a drop-in replacement for JSON.*, but it will ignore stringifiers/revivers. I updated the README, focusing solely on v2 and how to use it. The test fails because Jest doesn't work on Node v4 ;) I think it's in good shape for a v2.0.0-rc1 release. It's a breaking change, but downwards compatible, so people can choose to stay on v1. |
@bjouhier any thoughts on merging this? Otherwise I'm thinking to put this on npm as jsurl2… |
Jest is no longer compatible with Node 4, plus that's no longer maintained anyway
Some browsers don't like it inside strings in JS
* change cmp so it accepts objects for round-trip encode and checking assertions
@bjouhier I have it on npm in a separate package, is that what you would prefer? By making it a v2 release on Sage/jsurl it could help more people. The v2 would indicate that it's a breaking change. |
Awesome PR @wmertens. Exactly what I need just now 👍 |
Oh almost two years have passed and there is still only |
UPDATE: This is on npm https://www.npmjs.com/package/@yaska-eu/jsurl2
I implemented more-or-less the grammar I describe in #16 (comment).
You can see some examples of what it produces in the
test/jsurl2.js
file. Notably, it now can handle dates too, and it leaves URI encoding to whatever uses it next, while being immune to it.For backwards compatibility, the v1 could look at the string and see if it ends with ~, in which case it should call v2.
(I switched to AVA for tests because it makes iterative TDD so much nicer. I also removed the semicolons from the original tests via my formatter, apologies, I can figure out how to to put them back)
TODO:
.toJSON