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

Normative: Add Temporal[@@toStringTag]. #1541

Merged
merged 2 commits into from
Jul 27, 2021
Merged

Normative: Add Temporal[@@toStringTag]. #1541

merged 2 commits into from
Jul 27, 2021

Conversation

Ms2ger
Copy link
Collaborator

@Ms2ger Ms2ger commented Jun 17, 2021

Fixes #1539.

@codecov
Copy link

codecov bot commented Jun 17, 2021

Codecov Report

Merging #1541 (c740827) into main (828dd2f) will decrease coverage by 3.92%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1541      +/-   ##
==========================================
- Coverage   95.02%   91.09%   -3.93%     
==========================================
  Files          19       17       -2     
  Lines       10778    10774       -4     
  Branches     1725     1608     -117     
==========================================
- Hits        10242     9815     -427     
- Misses        523      943     +420     
- Partials       13       16       +3     
Flag Coverage Δ
test262 ?
tests 90.15% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
polyfill/lib/now.mjs 100.00% <100.00%> (ø)
polyfill/lib/plaindate.mjs 72.06% <0.00%> (-18.31%) ⬇️
polyfill/lib/plaintime.mjs 79.72% <0.00%> (-11.03%) ⬇️
polyfill/lib/plainmonthday.mjs 83.56% <0.00%> (-9.59%) ⬇️
polyfill/lib/zoneddatetime.mjs 90.62% <0.00%> (-7.77%) ⬇️
polyfill/lib/instant.mjs 87.15% <0.00%> (-7.30%) ⬇️
polyfill/lib/timezone.mjs 86.27% <0.00%> (-7.19%) ⬇️
polyfill/lib/plaindatetime.mjs 90.34% <0.00%> (-6.97%) ⬇️
polyfill/lib/plainyearmonth.mjs 89.77% <0.00%> (-6.63%) ⬇️
polyfill/lib/duration.mjs 93.65% <0.00%> (-4.43%) ⬇️
... and 5 more

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 828dd2f...c740827. Read the comment docs.

@Ms2ger Ms2ger added the needs plenary input Needs to be presented to the committee and feedback incorporated label Jun 17, 2021
Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will mark this as "draft" so it's not accidentally merged before being presented to plenary.

@ptomato ptomato marked this pull request as draft June 17, 2021 16:29
@ptomato
Copy link
Collaborator

ptomato commented Jun 23, 2021

If we go with Temporal.now[@@toStringTag] as discussed on #1539 then this PR will need to be updated.

@ptomato ptomato removed the needs plenary input Needs to be presented to the committee and feedback incorporated label Jul 27, 2021
@ptomato
Copy link
Collaborator

ptomato commented Jul 27, 2021

I believe this is covered by the consensus achieved at the July 2021 TC39 meeting as part of the "Guidance on nested namespaces" item. I will add the updates to Temporal.Now as per that item and un-draft this.

@ptomato ptomato marked this pull request as ready for review July 27, 2021 00:35
Ms2ger and others added 2 commits July 27, 2021 17:46
Fixes #1539.

Co-authored-by: Jordan Harband <ljharb@gmail.com>
As per the July 2021 TC39 meeting, this should have the value
"Temporal.Now".
@Ms2ger Ms2ger enabled auto-merge (rebase) July 27, 2021 15:46
@Ms2ger Ms2ger merged commit 0421e37 into main Jul 27, 2021
@Ms2ger Ms2ger deleted the toStringTag branch July 27, 2021 16:10
@justingrant
Copy link
Collaborator

Does this change need to be ported to @js-temporal/polyfill? shim.js doesn't exist over there because that polyfill doesn't (yet) put itself in the global namespace, so I wasn't sure how this PR fits in to it, if at all.

@Ms2ger
Copy link
Collaborator Author

Ms2ger commented Aug 27, 2021

Temporal is a module namespace object in that context, I think, and it might not be possible to put a Symbol on there. We'll need to figure out how / if we want to support exposing the object on the global object there.

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.

Should we have a Temporal [ @@toStringTag ] ??
4 participants