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.

Sign up for GitHub

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

Jump to bottom

Normative: nested property bags and ISO strings in Calendar/TimeZone.from #2485

Merged
merged 5 commits into from
Apr 10, 2023
Merged

Normative: nested property bags and ISO strings in Calendar/TimeZone.from #2485

merged 5 commits into from
Apr 10, 2023

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Jan 20, 2023 •
edited
Loading

Closes: #2104

Note, this PR is staged on top of #2482. For review purposes, look only at the last four commits, or look at this comparison.

@ptomato ptomato changed the title 2104 nested property bags iso strings Normative: nested property bags and ISO strings in Calendar/TimeZone.from Jan 20, 2023
@justingrant
Copy link
Collaborator

Could we make this a draft PR until we have consensus on desired behavior?

@ptomato ptomato marked this pull request as draft January 20, 2023 06:11
@ptomato ptomato mentioned this pull request Jan 25, 2023
Closed
@ptomato ptomato force-pushed the 2104-nested-property-bags-iso-strings branch 2 times, most recently from c7ee0f8 to c9b5ffc Compare January 28, 2023 02:30
@ptomato ptomato mentioned this pull request Jan 31, 2023
Closed
@ptomato ptomato mentioned this pull request Feb 24, 2023
Merged
@ptomato ptomato force-pushed the 2104-nested-property-bags-iso-strings branch from c9b5ffc to 4272b58 Compare March 6, 2023 22:14
@codecov
Copy link

codecov bot commented Mar 6, 2023 •
edited
Loading

Codecov Report

Merging #2485 (11a811f) into main (d26cf05) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2485      +/-   ##
==========================================
+ Coverage   95.91%   95.97%   +0.05%     
==========================================
  Files          20       20              
  Lines       11115    11123       +8     
  Branches     2114     2126      +12     
==========================================
+ Hits        10661    10675      +14     
+ Misses        392      387       -5     
+ Partials       62       61       -1
Impacted Files Coverage Δ
polyfill/lib/ecmascript.mjs 98.31% <100.00%> (+0.11%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ptomato ptomato mentioned this pull request Mar 23, 2023
Merged
justingrant
justingrant approved these changes Apr 4, 2023
View reviewed changes
Copy link
Collaborator

@justingrant justingrant left a comment

Choose a reason for hiding this comment

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

Looks good!

Ms2ger
Ms2ger approved these changes Apr 4, 2023
View reviewed changes
spec/timezone.html Outdated Show resolved Hide resolved
ptomato added a commit to ptomato/test262 that referenced this pull request Apr 7, 2023
@ptomato
Remove support for nested Temporal time zone property bags
fdc166f 
Previously, "nested" time zone property bags were unwrapped up to one
level. That is, this object:
{
  timeZone: {
     // ...Temporal.TimeZone methods
  }
}
would not be considered to implement the TimeZone protocol, but would have
its timeZone property used instead, if it were passed to an API that
required a TimeZone protocol object.

These nested property bags are no longer supported. Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to ptomato/test262 that referenced this pull request Apr 7, 2023
@ptomato
Remove support for nested Temporal calendar property bags
be84a7a 
Previously, "nested" calendar property bags were unwrapped up to one
level. That is, this object:
{
  calendar: {
     // ...Temporal.Calendar methods
  }
}
would not be considered to implement the Calendar protocol, but would have
its calendar property used instead, if it were passed to an API that
required a Calendar protocol object.

These nested property bags are no longer supported. Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to ptomato/test262 that referenced this pull request Apr 7, 2023
@ptomato
Validate required methods of Temporal TimeZone protocol
238483e 
Checking whether an object implements the TimeZone protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the TimeZone brand.

Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to ptomato/test262 that referenced this pull request Apr 7, 2023
@ptomato
Validate required methods of Temporal Calendar protocol
80b37da 
Checking whether an object implements the Calendar protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the Calendar brand.

Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
@ptomato ptomato mentioned this pull request Apr 7, 2023
Merged
@ptomato ptomato force-pushed the 2104-nested-property-bags-iso-strings branch from 4272b58 to e243b13 Compare April 7, 2023 19:42
@ptomato
Copy link
Collaborator Author

ptomato commented Apr 7, 2023

Thanks for the reviews. For the record, this PR got consensus in the January/February 2023 TC39 meeting. The only remaining obstacle for merging it is test262 tests, which I've just opened a PR for: tc39/test262#3813

ptomato added a commit to ptomato/test262 that referenced this pull request Apr 10, 2023
@ptomato
Remove support for nested Temporal time zone property bags
52062ed 
Previously, "nested" time zone property bags were unwrapped up to one
level. That is, this object:
{
  timeZone: {
     // ...Temporal.TimeZone methods
  }
}
would not be considered to implement the TimeZone protocol, but would have
its timeZone property used instead, if it were passed to an API that
required a TimeZone protocol object.

These nested property bags are no longer supported. Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to ptomato/test262 that referenced this pull request Apr 10, 2023
@ptomato
Remove support for nested Temporal calendar property bags
a4068a0 
Previously, "nested" calendar property bags were unwrapped up to one
level. That is, this object:
{
  calendar: {
     // ...Temporal.Calendar methods
  }
}
would not be considered to implement the Calendar protocol, but would have
its calendar property used instead, if it were passed to an API that
required a Calendar protocol object.

These nested property bags are no longer supported. Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to ptomato/test262 that referenced this pull request Apr 10, 2023
@ptomato
Validate required methods of Temporal TimeZone protocol
44c64db 
Checking whether an object implements the TimeZone protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the TimeZone brand.

Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to ptomato/test262 that referenced this pull request Apr 10, 2023
@ptomato
Validate required methods of Temporal Calendar protocol
c45c0f6 
Checking whether an object implements the Calendar protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the Calendar brand.

Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
@ptomato
Normative: Remove support for nested time zone property bags
d1f5d70 
Previously, "nested" time zone property bags were unwrapped up to one
level. That is, this object:
{
  timeZone: {
     // ...Temporal.TimeZone methods
  }
}
would not be considered to implement the TimeZone protocol, but would have
its timeZone property used instead, if it were passed to an API that
required a TimeZone protocol object.

These nested property bags are no longer supported. Discussion:
#2104 (comment)

See: #2104
@ptomato
Normative: Remove support for nested calendar property bags
87f6924 
Previously, "nested" calendar property bags were unwrapped up to one
level. That is, this object:
{
  calendar: {
     // ...Temporal.Calendar methods
  }
}
would not be considered to implement the Calendar protocol, but would have
its calendar property used instead, if it were passed to an API that
required a Calendar protocol object.

These nested property bags are no longer supported. Discussion:
#2104 (comment)

See: #2104
@ptomato
Normative: Validate required methods of TimeZone protocol
4fc34cf 
Checking whether an object implements the TimeZone protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the TimeZone brand.

Discussion:
#2104 (comment)

See: #2104
@ptomato
Normative: Validate required methods of Calendar protocol
53c3dc2 
Checking whether an object implements the Calendar protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the Calendar brand.

Discussion:
#2104 (comment)

See: #2104
ptomato added a commit to tc39/test262 that referenced this pull request Apr 10, 2023
@ptomato
Remove support for nested Temporal time zone property bags
24def91 
Previously, "nested" time zone property bags were unwrapped up to one
level. That is, this object:
{
  timeZone: {
     // ...Temporal.TimeZone methods
  }
}
would not be considered to implement the TimeZone protocol, but would have
its timeZone property used instead, if it were passed to an API that
required a TimeZone protocol object.

These nested property bags are no longer supported. Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to tc39/test262 that referenced this pull request Apr 10, 2023
@ptomato
Remove support for nested Temporal calendar property bags
bc979c5 
Previously, "nested" calendar property bags were unwrapped up to one
level. That is, this object:
{
  calendar: {
     // ...Temporal.Calendar methods
  }
}
would not be considered to implement the Calendar protocol, but would have
its calendar property used instead, if it were passed to an API that
required a Calendar protocol object.

These nested property bags are no longer supported. Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to tc39/test262 that referenced this pull request Apr 10, 2023
@ptomato
Validate required methods of Temporal TimeZone protocol
d6a24fe 
Checking whether an object implements the TimeZone protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the TimeZone brand.

Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
ptomato added a commit to tc39/test262 that referenced this pull request Apr 10, 2023
@ptomato
Validate required methods of Temporal Calendar protocol
33865c5 
Checking whether an object implements the Calendar protocol is now done by
means of HasProperty operations for each of the required methods unless
the object already has the Calendar brand.

Discussion:
tc39/proposal-temporal#2104 (comment)

Corresponding normative PR:
tc39/proposal-temporal#2485
@ptomato ptomato force-pushed the 2104-nested-property-bags-iso-strings branch from e243b13 to 11a811f Compare April 10, 2023 15:38
@ptomato ptomato marked this pull request as ready for review April 10, 2023 15:40
@ptomato ptomato merged commit 483aca8 into main Apr 10, 2023
@ptomato ptomato deleted the 2104-nested-property-bags-iso-strings branch April 10, 2023 15:45
ptomato added a commit that referenced this pull request Apr 28, 2023
@ptomato
Editorial: Remove doubled spec step
78ddcfe 
I believe this was a rebase error in #2485 which I didn't catch in review.
ptomato added a commit that referenced this pull request May 2, 2023
@ptomato
Editorial: Remove doubled spec step
71f7b74 
I believe this was a rebase error in #2485 which I didn't catch in review.
ptomato added a commit that referenced this pull request May 11, 2023
@ptomato
Editorial: Remove doubled spec step
e98d636 
I believe this was a rebase error in #2485 which I didn't catch in review.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 18, 2023
@anba
Bug 1840374 - Part 8: Implement changes for ToTemporalTimeZoneSlotVal…
6b7e402 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182025

Differential Revision: https://phabricator.services.mozilla.com/D182026
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 18, 2023
@anba
Bug 1840374 - Part 9: Implement changes for ToTemporalCalendarSlotVal…
c3a1c92 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182026

Differential Revision: https://phabricator.services.mozilla.com/D182027
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Jul 19, 2023
@anba
Bug 1840374 - Part 8: Implement changes for ToTemporalTimeZoneSlotVal…
e57c9e2 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182025

Differential Revision: https://phabricator.services.mozilla.com/D182026
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Jul 19, 2023
@anba
Bug 1840374 - Part 9: Implement changes for ToTemporalCalendarSlotVal…
50520d4 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182026

Differential Revision: https://phabricator.services.mozilla.com/D182027
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Jul 21, 2023
@marco-c
Bug 1840374 - Part 8: Implement changes for ToTemporalTimeZoneSlotVal…
43f4686 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182025

Differential Revision: https://phabricator.services.mozilla.com/D182026

UltraBlame original commit: a26950114649f104836074e06d726358ba94c048
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Jul 21, 2023
@marco-c
Bug 1840374 - Part 9: Implement changes for ToTemporalCalendarSlotVal…
7b4abaa 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182026

Differential Revision: https://phabricator.services.mozilla.com/D182027

UltraBlame original commit: 12e79d0c699efde090bcded208f9585c35dfd856
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Jul 21, 2023
@marco-c
Bug 1840374 - Part 8: Implement changes for ToTemporalTimeZoneSlotVal…
95d8417 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182025

Differential Revision: https://phabricator.services.mozilla.com/D182026

UltraBlame original commit: a26950114649f104836074e06d726358ba94c048
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Jul 21, 2023
@marco-c
Bug 1840374 - Part 9: Implement changes for ToTemporalCalendarSlotVal…
f88d79d 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182026

Differential Revision: https://phabricator.services.mozilla.com/D182027

UltraBlame original commit: 12e79d0c699efde090bcded208f9585c35dfd856
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Jul 21, 2023
@marco-c
Bug 1840374 - Part 8: Implement changes for ToTemporalTimeZoneSlotVal…
b78c309 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182025

Differential Revision: https://phabricator.services.mozilla.com/D182026

UltraBlame original commit: a26950114649f104836074e06d726358ba94c048
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Jul 21, 2023
@marco-c
Bug 1840374 - Part 9: Implement changes for ToTemporalCalendarSlotVal…
219cf5a 
…ue. r=spidermonkey-reviewers,sfink

Implement the changes from <tc39/proposal-temporal#2485>.

The function name hasn't yet been updated to reflect the new name.

Depends on D182026

Differential Revision: https://phabricator.services.mozilla.com/D182027

UltraBlame original commit: 12e79d0c699efde090bcded208f9585c35dfd856
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justingrant justingrant justingrant approved these changes

@Ms2ger Ms2ger Ms2ger approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Recursive calendar or timeZone property not rejected if property value is a Temporal object
3 participants
@ptomato @justingrant @Ms2ger