-
Notifications
You must be signed in to change notification settings - Fork 10
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
So the title basically says it all as far as implementation. That being said, there's a lot of interpretation going into this that I'd like feedback on if possible, and it could be the right direction or it could be the wrong direction (although I'm leaning the former over the latter). The main culprit is basically [PrepareTemporalFields](https://tc39.es/proposal-temporal/#sec-temporal-preparetemporalfields). Background for discussion/sanity check: Up until recently, I basically thought it would be an implementation detail on the engine/interpreter side, but the thing that always bugged me was the `requiredFields` parameter, being either a List or PARTIAL. We could probably do that to specification, but we might be providing something like `Some(Vec::default())` as an argument, and it basically just felt clunky. After the recent `TemporalFields` update, I went to implement the `TemporalFields` portion of the `toX` abstract ops in Boa and realized that PARTIAL is never called in the `toX` operations, and it's actually exclusively called in `with` methods. We already have a sort of precedence for partials with `PartialDuration`. There's some benefits to this: we can have a with method on the native rust side, ideally the complexity that exists in `PrepareTemporalFields` can be made a bit easier to reason about. Potential negatives: we might end up deviating from the specification as far as the order of when errors are thrown and observability (TBD...potentially a total non-issue) and this is probably opening up a can of worms around what would be the ideal API for a `PartialDate`, `PartialDateTime`, and `PartialTime`. That all being said, I think the benefits do most likely outweigh any negatives, and it would be really cool to have `with` method implementations. I'm just not entirely sure around the API. Also, there's an addition of a `MonthCode` enum to make `From<X> for TemporalFields` implementations easier.
- Loading branch information
Showing
4 changed files
with
325 additions
and
73 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.