Skip to content

RFC: Fix null-check stuff #103

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

Closed
wants to merge 3 commits into from
Closed

RFC: Fix null-check stuff #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52a048a
fix ts issues in plaindate
jens-ox Dec 7, 2021
ad911d8
remove getIntrinsic
jens-ox Dec 7, 2021
5aec4ed
Merge branch 'main' of github.com:jens-ox/temporal-polyfill into ts-i…
jens-ox Dec 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ tsc-out/
.vscode/*
!.vscode/launch.json
*.tgz
.DS_STORE
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a newline at the end of this line. I think npm run fix should automatically add it. If it doesn't, then you might want to tweak the npm run fix command line args. 😄

8 changes: 4 additions & 4 deletions lib/calendar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { DEBUG } from './debug';
import * as ES from './ecmascript';
import { GetIntrinsic, MakeIntrinsicClass, DefineIntrinsic } from './intrinsicclass';
import { MakeIntrinsicClass, DefineIntrinsic } from './intrinsicclass';
import {
CALENDAR_ID,
ISO_YEAR,
Expand Down Expand Up @@ -28,6 +28,7 @@ import type {
CalendarReturn as Return,
FieldRecord
} from './internaltypes';
import { Duration } from './duration';

const ArrayIncludes = Array.prototype.includes;
const ArrayPrototypePush = Array.prototype.push;
Expand Down Expand Up @@ -117,7 +118,7 @@ export class Calendar implements Temporal.Calendar {
return ES.ToString(this);
}
dateFromFields(
fields: Params['dateFromFields'][0],
fields?: Params['dateFromFields'][0],
Copy link
Contributor

Choose a reason for hiding this comment

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

fields should never be undefined.

Also, if undefined were acceptable, then for any public API (like this one) it's intentional to leave it as Params... because that enforces alignment between the implementation and index.d.ts. So let's imagine that undefined were OK for this API, then the right fix would be to change index.d.ts, not here.

optionsParam: Params['dateFromFields'][1] = undefined
): Return['dateFromFields'] {
if (!ES.IsTemporalCalendar(this)) throw new TypeError('invalid receiver');
Expand Down Expand Up @@ -178,7 +179,7 @@ export class Calendar implements Temporal.Calendar {
if (!ES.IsTemporalCalendar(this)) throw new TypeError('invalid receiver');
const date = ES.ToTemporalDate(dateParam);
const duration = ES.ToTemporalDuration(durationParam);
const options = ES.GetOptionsObject(optionsParam);
const options = ES.GetOptionsObject<Temporal.ArithmeticOptions>(optionsParam);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this explicit type parameter needed? Can't the type be inferred by TS from the type of options ?

const overflow = ES.ToTemporalOverflow(options);
const { days } = ES.BalanceDuration(
GetSlot(duration, DAYS),
Expand Down Expand Up @@ -216,7 +217,6 @@ export class Calendar implements Temporal.Calendar {
'day'
);
const { years, months, weeks, days } = impl[GetSlot(this, CALENDAR_ID)].dateUntil(one, two, largestUnit);
const Duration = GetIntrinsic('%Temporal.Duration%');
return new Duration(years, months, weeks, days, 0, 0, 0, 0, 0, 0);
}
year(dateParam: Params['year'][0]): Return['year'] {
Expand Down
Loading