Skip to content

Editorial: Close more old non-normative issues #2815

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

Merged
merged 4 commits into from
Apr 3, 2024
Merged

Editorial: Close more old non-normative issues #2815

merged 4 commits into from
Apr 3, 2024

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Apr 3, 2024

@ptomato ptomato requested review from justingrant and Ms2ger April 3, 2024 01:00
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 94.96855% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 96.56%. Comparing base (837f6a3) to head (a5a4fc3).

Files Patch % Lines
polyfill/lib/intl.mjs 96.57% 5 Missing ⚠️
polyfill/lib/ecmascript.mjs 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2815      +/-   ##
==========================================
+ Coverage   96.49%   96.56%   +0.06%     
==========================================
  Files          23       23              
  Lines       12304    12386      +82     
  Branches     2272     2298      +26     
==========================================
+ Hits        11873    11960      +87     
+ Misses        372      364       -8     
- Partials       59       62       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ptomato added 4 commits April 3, 2024 10:47
The Set specification type's definition says that it is for use in the
memory model. Rather than open the can of worms to extend the definition,
just use a List and remove duplicates from it before returning it.

Closes: #2531
The time zone does not affect the `daysInMonth` value even if a 24-hour
UTC offset shift causes a skipped day, so this claim is false.

Closes: #2495
…ties

This matches the other type assertions in es-abstract.
Previously we did not care very much about fidelity for the replacement
Intl.DateTimeFormat object, since the point of this code is to run the
Temporal tests. Occasionally people want to run the Intl.DateTimeFormat
tests and are surprised that it doesn't work.

Things we have to do to get the Intl.DateTimeFormat tests to pass:

- Change ASCIILowercase not to use a RegExp, so that it doesn't clobber
  legacy RegExp variables like RegExp.$_. (Probably this means that for a
  100% conformant polyfill, you can't use any RegExp anywhere. I'm not
  bothering with that.)

- Change DateTimeFormat to be a class, DateTimeFormatImpl. Methods defined
  in a class body don't have a 'prototype' property and can't be called as
  constructors, whereas regular functions do.
  (However, we still need a regular function to construct DateTimeFormat,
  because it needs to be able to be invoked without 'new'.)

- Use CreateSlots/GetSlot/SetSlot like the other Temporal objects, instead
  of symbol-valued properties. The symbol-valued properties were
  observably present for user code.

- In the constructor, observably read all the options from the options bag
  in the expected order, and copy them to a null-prototype object with
  which we do subsequent operations unobservably.

- Implement the behaviour of format() already having bound the correct
  this-object.

- Previously we only half-implemented the normative optional behaviour of
  being able to 'bless' a plain object into becoming a DateTimeFormat
  object.
  Instead, just don't implement it at all. (A real production polyfill
  should probably check if the existing Intl.DateTimeFormat has that
  behaviour, and match that.)

Closes: #2471
@ptomato ptomato merged commit 985b826 into main Apr 3, 2024
9 checks passed
@ptomato ptomato deleted the editorial branch April 3, 2024 17:51
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.

2 participants