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

revisit: Dealing with JSON.stringify() #80

Closed
kaizhu256 opened this issue Aug 5, 2018 · 20 comments
Closed

revisit: Dealing with JSON.stringify() #80

kaizhu256 opened this issue Aug 5, 2018 · 20 comments

Comments

@kaizhu256
Copy link
Contributor

#77 is vague.
looking at README.md, these would be the results under JSON.stringify? does the behavior essentially copy toString()?

JSON.stringify(CivilDate)     // '2017-12-31'
JSON.stringify(CivilTime)     // '17:00:00'
JSON.stringify(CivilDateTime) // '2017-12-31T12:00:00'
JSON.stringify(Instant)       // '2017-12-31T00:00:00Z'
JSON.stringify(ZonedInstant)  // '2017‑12‑31T09:00:00+09:00[Asia/Tokyo]'
@pipobscure
Copy link
Collaborator

pipobscure commented Aug 15, 2018

That is pretty much it, except that

JSON.stringify(ZonedInstant) // '2017‑12‑31T09:00:00+09:00' since the geographic name is problematic especially since it can be in conflict with the offset. Also a full geographic DB may not be available (think IOT).


Edit (2018-09-20): We have reversed that. The geographic IANA name has become optional, but will be there if known.

@kaizhu256
Copy link
Contributor Author

what would be the roundtrip/revive solution? would it follow Date's behavior and have the constructors accept a JSON-string as a first-argument?

// copy Date-like roundtrip behavior?
date1 === new Date(JSON.parse(JSON.stringify(date1));

civilDate1 === new CivilDate(JSON.parse(JSON.stringify(civilDate1));
civilTime1 === new CivilTime(JSON.parse(JSON.stringify(civilTime1));
civilDateTime1 === new CivilDateTime(JSON.parse(JSON.stringify(civilDateTime1));
instant1 === new Instant(JSON.parse(JSON.stringify(instant1));
zonedInstant1 === new ZonedInstant(JSON.parse(JSON.stringify(zonedInstant1));

@pipobscure
Copy link
Collaborator

@kaizhu256 no, the constructors would only be:

  • CivilDate(year: number, month: number, day: number)
  • CivilTime(hours: number, minutes: number, seconds: number, milliseconds: number = 0, nanoseconds: number = 0)
  • CivilDate(year: number, month: number, day: number, hours: number, minutes: number, seconds: number, milliseconds: number = 0, nanoseconds: number = 0)
  • Instant(nanoseconds: BigInt)
  • ZonedInstant(instant: Instant, zone: string)

However there are .fromString(iso: string), fromMilliseconds(epochms: numeric), etc.

@kaizhu256
Copy link
Contributor Author

kaizhu256 commented Sep 19, 2018

why not? are there technical reasons? this seems to make CivilObjects less easy-to-use than Date for web-developers dealing with product-integration.

@kaizhu256 kaizhu256 reopened this Sep 19, 2018
@pipobscure
Copy link
Collaborator

pipobscure commented Sep 19, 2018 via email

@kaizhu256
Copy link
Contributor Author

this is a step-backwards from Date in terms of ease-of-use. you do realize that unlike other languages, most real-world use-cases for data-structures in javascript product-development revolve around JSON serialization/revival?

@kaizhu256
Copy link
Contributor Author

from a use-case perspective, javascript-users are far more likely to construct these CivilObjects from JSON/url input-strings (via server-http-requests, dom-element-data-attributes, client-ajax-responses) than through arguments-based constructors. you're calling for a design that ignores the common-case scenario in web-development.

@ljharb
Copy link
Member

ljharb commented Sep 19, 2018

most real-world use-cases for data-structures in javascript product-development revolve around JSON serialization/revival?

That's not actually the case, and it's a very bold claim backed by no evidence - and it doesn't match Airbnb's use case, as an example.

Similarly, "ease of use", while important, is far less important than "correct" - Date is not correct by many metrics, however Temporal will be.

@pipobscure
Copy link
Collaborator

pipobscure commented Sep 19, 2018 via email

@kaizhu256
Copy link
Contributor Author

@pipobscure. yes. i'm saying your arguments-based constructors are a minority use-case, when most web-based inputs are string-based.

@ljharb, i don't believe its a strong-statement. most of javascript programming i've experienced in industry revolves around string-manipulation of dom/url/request/response data/properties (and then directly passing manipulated-strings back to client/server/db/dom).

class-instantiation of things like CivilObjects are for mostly transient-objects, that are quickly destroyed, once the client <-> server <-> db request/response is done. alot of these real-world tasks could be acoomplished more efficiently, with just static-functions that manipulate strings like (#82) and (#53)

@rxaviers
Copy link
Member

rxaviers commented Sep 19, 2018

In order to foster a productive discussion, I suggest we focus on explaining the design principles of this proposal vs. what @kaizhu256 asked for. For example, someone involved in the proposal could comment about the reasons/benefits/principles behind choosing the static Civil*.fromString() method instead of allowing multiple constructor signatures. That's ultimately what's "prohibiting" @kaizhu256 to accomplish what he wants above, right (unless I am missing something)?

@pipobscure
Copy link
Collaborator

pipobscure commented Sep 20, 2018

Ok,
here are some of the thoughts that went into that:

  1. The different variants of the Date constructor have always been a cause for confusion. This goes so far as to cause many people to not use the string variant at all, but rather use Date.parse. (so ist the new Date(y, m-1, d, ...) variant. Use new Date(Date.UTC()) instead if you can [timezones are hard]).
  2. The support / lack of calling the constructor without new is another issue that makes it questionable what the correct behaviour should be.
  3. The Civil* constructors are all intended to be rarely used, since they are far less explicit than the alternatives.
    • .fromString() - makes it explicit that you are creating an instance from a string and will throw on other input.
    • .fromZonedInsstant() - likewise makes it explicit that you are creating this from an instance of ZonedInstance and will throw otherwise.

The most frequent task in creating software of any kind is actually NOT writing code, but rather reading it. Therefore being explicit is much more valuable.

Now if one is insistent on always using constructors, the conversation to be had isn't so much about the stringify round-trip (which is important), but rather about whether the use of constructors for that purpose is a good thing™️. I would go so far as to claim that new Date(JSON.stringify(date)) is an anti-pattern, and what you should have been doing instead is Date.parse(JSON.stringify(date)).


but first things last: while the serialisation/deserialisation round-trip is a quite important use-case, it's by no means the dominant one. It may be the dominant one in a pure web-browser world, but even that premise is arguable. From your reference to #82 & #53 I deduce a clear preference for basically doing everything in the world with just strings (I'm exaggerating obviously); If that's the case, there is nothing to prevent you to wrap temporal in a very thin layer to do that for you; I'll even voluteer to write you that wrapper to add .datePlus() and .timePlus() to the Date-object in terms of temporal. However I do have to tell you that I would never condone doing that. It's what I would consider an anti-pattern and you can find some of the reasons in these issues.


last but not least:

I truly do not comprehend your argument that new CivilDate(JSON.stringify(date)) is in any way easier than CivilDate.fromString(JSON.stringify(date)). Please make me understand your reasoning. To me the second is more explicit while the first causes parameter-overload-pollution, but otherwise they're the same thing. I feel I must be missing something.

You say that "arguments-based constructors are a minority use-case", and I'm saying that "multivariant constructors are an anti-pattern" and "your use-case is just as easy using what we propose, just not using a constructor". What is it that eludes me?

@kaizhu256
Copy link
Contributor Author

why are new Date(JSON.stringify(date)), Date.datePlus, Date.timePlus, and multivariant constructors anti-patterns? javascript is not a low-level, general-purpose programming-language like c#/java/c++. its use-case in industry is generally in a mutually-exclusive domain from those languages (high-level web-integration via JSON/urlstring between dom<->client<->server<->database, vs low-level library-code).

if you want to say its an anti-pattern, can you give examples in a javascript-context (rather than a c#/java/c++ context) on why its bad?

@pipobscure
Copy link
Collaborator

pipobscure commented Sep 20, 2018

I'll repeat the question: what am I missing; but ok here we go:


First off:

Multivariant Constructor Anti-Pattern

Clarity / Explicitness

For one thing multivariant constructors are bad because it is hard to know what kinds of arguments you are expecting. Given that JS is dynamically typed, this can become problematic because mistyped arguments may still be acceptable, but for another variant of the constructor.

If one instead uses explicit static functions, that woe goes away. Date.parse() is more explict in that it at least makes it clear that it's a piece of data that is to be parsed.

The Civil* variants are even more explicit. CivilDate.fromString() explicitly state that it's expecting a string argument and that it's creating a CivilDate.

Variant Variability causes trouble with dynamic types

Of course the unexpected types may well lead to a different constructor variant being used producing valid objects with unexpected values.

Examples:

new Date(1998); // 1970-01-01T00:00:01.998Z
new Date("1998"); // 1998-01-01T00:00:00.000Z
new Date([ 1998 ]); // 1998-01-01T00:00:00.000Z
new Date([ "1998" ]); // 1998-01-01T00:00:00.000Z

These results (using v8 6.7.288.46) are utterly unintuitive for non-literal arguments.

Take these instead:

CivilDate.fromString("1998"); // throws
CivilDate.fromString(1998); // throws
CivilDate.fromString([ 1998 ]); // throws
CivilDate.fromString([ "1998" ]); // throws
CivilDate.fromString("1998-01-15"); // valid 1998-01-15

beyond being just explicit, this makes it impossible to get corrupted data from bad arguments.


These are exactly the kinds of bugs that are pervasive. Multivariant constructors aren't the only cause of this of course; at heart it's part due to the way JS does type-coercion. But multivariant constructors make the issue worse.

In other words, especially in a dynamically typed language such as JavaScript being explicit is incredibly important. And that's before going into the fact that code-reading is actually much more pervasive than code-writing; as such being explicit is incredibly important and help people actually understand code better.


Second:

Stringly-Typed functions

serialise/deserialise overhead

Every action such as Date.datePlus() and Date.timePlus() requires the following three steps.

  1. Parse the string into a calculable form
  2. Do the calulation
  3. Serialise the result into a string

Rarely will any such calculation be used in isolation. As a simple example let's look at the following:

Date.timePlus(Date.datePlus("1998-09-23T00:00:00.000Z", { years: 10, months: -1 }), { hours: 3, minutes: 5 });

If you take the steps here, they'll be:

  1. Parse string into data-representation
  2. Do date-calculation
  3. Serialise into a string
  4. Parse string into data-representation
  5. Do time-calculation
  6. Serialise into a string

You will note that steps 3 & 4 are entirely superfluous. Now we could of course then just add a Date.datetimePlus() function, but that's a strawman. In truth the idea of a programming language is to create combinations of operations with it that were not necessarily thought of before. Adding unnecessary overhead here is not a good-idea.

As you mentioned yourself in #82, MDN warns against stringly typing for complex data. You then asserted that date-string is not complex-data. I consider that statement to be false. Of course dates and times are not the most complex data there is, but they are nevertheless complex data; arguable more complex than comma-separated lists. So I'll refer you back to the arguments made by the MDN page you cited with the presumption that dates/times are actually complex data. All of those will hold and give you further arguments for why Date.timePlus() and Date.datePlus() are actually a bad idea.


Lastly: I consider the claim "javascript is not a low-level, general-purpose programming-language like c#/java/c++." highly outdated. It may have held true in 1995 when it was created as a specialty language for web-browsers, but it hasn't been true since 1997 when Rhino was introduced as JS on the backend; no matter it's definitely become incorrect since 2009 with the advent of nodejs.

It may not be low-level if you take that to mean compiled directly to processor instructions then yeah. But as soon as you involve a runtime interpreter (such as a JVM or .NET) this becomes questionable.

But it definitely has become general-purpose and as low-level as Java/C# and the like. JS is currently being used to drive web-sites and web-servers; iot-microservices and iot-devices; shells and shell-scripts. The language is evolving to accomodate many, many more uses than "your grand-dads-javascript" and this proposal should be suited to all of them.

@rxaviers
Copy link
Member

@pipobscure thank you very much for responding to @kaizhu256

@kaizhu256
Copy link
Contributor Author

  • i accept the argument about multivariant constructor. i think CivilXxx.fromString might be acceptable.

  • i don't accept the argument about serialise/deserialise overhead. here's a performance benchmark done @ ISOString with static-functions as alternative to CivilObjects #53 (comment) to refute that claim.
    tldr, it benchmarks a simple, polyfill static-function implementation of Date.isoStringInstantGetMilliseconds that performs ~1million ops/second (on a lowly macbook-retina) doing the 6-step parse/calculate/serialize operation you say is too "expensive".

  • we disagree fundamentally on javascript's philosophy. javascript is a terrible language for low-level general-purpose programming. 5 years from now, es15/flow/typescript will likely still underwhelm as a general-purpose language, and industry would likely still prefer c#/java/c++/etc as more cost-effective tools than javascript in that field (and continue hiring js-devs primarily for web/ux integration-tasks - iot included).

in truth the idea of a programming language is to create combinations of operations with it that were not necessarily thought of before.

that's exactly why we have javascript-fatigue now. tc39 carried that paradigm too far, and the result is chaos. this temporal-proposal is the kind of radical-change (rather than iteratively enhancing Date) that makes people like me feel tc39 has learned nothing from its past mistakes.

@ljharb
Copy link
Member

ljharb commented Sep 20, 2018

Date is inherently broken; the mistake would be iterating on it.

@kaizhu256
Copy link
Contributor Author

then can you demonstrate how this proposal is less-broken than Date? like filling out the (https://github.com/tc39/proposal-temporal#scenario-based-examples) section in the README with A/B code-examples, showing web-developers how CivilObjects would be easier-to-use than Date.

@rxaviers
Copy link
Member

Conclusion: "roundtrip/revive solution" is CivilDate.fromString(JSON.stringify(date)) (or whatever CivilDate becomes, e.g., GregorianDate). My understanding is that everyone here agreed that using "CivilXxx.fromString" is better than having multiple constructor signatures. Unless any ongoing discussion would change this conclusion, I think this issue could be closed...

I suggest other issues to be created to treat/discuss other topics. For example, "section in the README with A/B code-examples", @kaizhu256 I think that's indeed great to have. Could you go ahead and create a separate issue?

@kaizhu256
Copy link
Contributor Author

closing this issue per request.

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

No branches or pull requests

4 participants