Cookbook example for compiling time zone from tzdata rules

[ptomato]

An example of parsing the rules from the tzdata rules (sometimes called "Olson database") directly, and creating a Temporal.TimeZone object out of them. By far the longest cookbook example.

A few additional bug fixes in the polyfill, as well.

Closes: #605

[codecov]

Codecov Report

Merging #1370 (4fc9cf5) into main (d51828d) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1370   +/-   ##
=======================================
  Coverage   95.36%   95.36%           
=======================================
  Files          19       19           
  Lines       11123    11123           
  Branches     1826     1826           
=======================================
  Hits        10607    10607           
  Misses        511      511           
  Partials        5        5           

Flag	Coverage Δ
test262	48.45% <0.00%> (ø)
tests	91.06% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
polyfill/lib/ecmascript.mjs	96.08% <100.00%> (ø)

---

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 d51828d...4fc9cf5. Read the comment docs.

[ptomato]

Huh, I thought we had private fields in Node 12, but I guess it was literally only fields and not methods. I'll rewrite the example not to use them.

[ljharb]

Private methods and accessors only landed in node 14 something.

[cjtenny]

Pending moving the ES.TemporalDurationToString fixes out from here per #1375, lgtm! (only separate them if you still want to - I'm fine shipping them as part of this).

I think it could be nice adding a quick test case for Pacific/Apia's day skip as well since we reference that example elsewhere in the docs a lot, but it's fine without it too.

[cjtenny]

Any reason to this is split into a separate md file? The other examples all embed directly.

[ptomato]

Two reasons: one is that these last few examples are much larger than the other cookbook snippets, so for practicality they can live on their own pages. The other reason is that these last few examples are not so much intended as "copy this recipe if you want to do X" but more like "here's something esoteric that Temporal is capable of, that you probably won't need to do, but you might adapt this recipe for something else".

[gilmoreorless]

Is this intended to be a general-purpose tzdata parser, or just the minimum needed for the America/Chicago example? I only ask because I can see it will fail for some other zones' source data. But I wanted to know the purpose before digging into specifics.

[ptomato]

I think it could be nice adding a quick test case for Pacific/Apia's day skip as well since we reference that example elsewhere in the docs a lot, but it's fine without it too.

This is done, and it uncovered several bugs. So thanks for the suggestion. For ease of review, here's a list of what I had to change:

• added Pacific/Apia tzdata records, and tests for them
• fixed bug in getRuleSetInEffect() not calculating the correct instant in which the state ends (added standard offset instead of subtracting)
• started cacheing dstShift in the cache records as well, because it's necessary when considering the first DST rule change during a calendar year (previously I erroneously assumed it to be 0)
• if the state ends before the DST rule ends, cache the state ending time instead of the DST rule ending time
• don't fall through to next year's DST transitions if the state ends this year

Is this intended to be a general-purpose tzdata parser, or just the minimum needed for the America/Chicago example? I only ask because I can see it will fail for some other zones' source data. But I wanted to know the purpose before digging into specifics.

I wouldn't go so far as to claim that it's general purpose; that said, what does it fail on? I'd like to investigate that, even if not now then some other time. I have actually measured getNextTransition()/getPreviousTransition() to be faster using this code than the polyfill's current approach of using Intl to get the offset and bisecting, so it might come in handy when working on a production-quality polyfill.

[gilmoreorless]

I wouldn't go so far as to claim that it's general purpose; that said, what does it fail on? I'd like to investigate that, even if not now then some other time.

I tried it with the Europe/Dublin data, knowing that that zone has caused problems for other parsers recently (e.g. Java's tzupdater broke when "negative DST" was introduced). It looks like some of the bugs I saw have been fixed by your latest changes.

A couple of things still remain, though. The RULES column of a Zone definition can contain a UTC offset instead of a named Rule. (The Pacific/Honolulu example at the bottom of the referenced how-to file also shows this case).

Zone	Europe/Dublin	-0:25:00 -	LMT	1880 Aug  2
			-0:25:21 -	DMT	1916 May 21  2:00s
			-0:25:21 1:00	IST	1916 Oct  1  2:00s
			 0:00	GB-Eire	%s	1921 Dec  6 # independence

The third line has a rule of 1:00 as a shortcut for "add one hour to the standard offset", which isn't currently handled. The code throws TypeError: ruleSet is not iterable.

There's also a subtle bug lurking in that example above. The s or u suffix on the "until" time is handled when parsing a Rule, but not a Zone. When the time ends with :00s, the code still works because 00s gets coerced to NaN during parsing, but that then falls back to the default 0 when being passed into PlainDateTime. But when the suffix is u, the behaviour is incorrect. A good test for this is Atlantic/Azores based on its second line:

Zone Atlantic/Azores	-1:42:40 -	LMT	1884        # Ponta Delgada
			-1:54:32 -	HMT	1912 Jan  1  2:00u # Horta MT

I have actually measured getNextTransition()/getPreviousTransition() to be faster using this code than the polyfill's current approach of using Intl to get the offset and bisecting, so it might come in handy when working on a production-quality polyfill.

Agreed on this. I noticed the difference when stepping through the code in a debugger while trying to pinpoint the case above. This new code is definitely faster.

[cjtenny]

Sorry for forgetting about this one; I didn't previously re-review because of @gilmoreorless' assertion that some bugs remain, but then I forgot about it for quite a while as a result. If it's not intended to be general and just an example that works only with the included data, then I'm still good with shipping it! No new comments on the changes since prior review, still looks good to me.

[ptomato]

Oh, I am still planning to address those, but it's on a lower priority right now until we get all the changes from the March TC39 plenary landed.