Skip to content

Commit

Permalink
Follow-up from code review
Browse files Browse the repository at this point in the history
* Ensure Intl.DateTimeFormat.length === 0
* Split out timezone lazy-init special case to a separate function
* Deep-clone inputs to prevent mutation before lazy instance creation,
  and add tests to verify that mutating inputs doesn't break anything.
* Add timing into npm test script
  • Loading branch information
justingrant committed Jul 12, 2021
1 parent bfac998 commit b268b7a
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 18 deletions.
44 changes: 30 additions & 14 deletions lib/intl.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -48,24 +48,40 @@ const ObjectAssign = Object.assign;
// See https://bugs.chromium.org/p/v8/issues/detail?id=6528
function getPropLazy(obj, prop) {
let val = obj[prop];
if (prop === TZ_RESOLVED) {
// for TimeZone, the initializer is a string, not a function
if (typeof val === 'string') {
val = new TimeZone(val);
obj[prop] = val;
}
} else if (typeof val === 'function') {
if (typeof val === 'function') {
val = new IntlDateTimeFormat(obj[LOCALE], val(obj[OPTIONS]));
obj[prop] = val;
}
return val;
}
// Similarly, lazy-init TimeZone instances.
function getResolvedTimeZoneLazy(obj) {
let val = obj[TZ_RESOLVED];
if (typeof val === 'string') {
val = new TimeZone(val);
obj[TZ_RESOLVED] = val;
}
return val;
}

function deepClone(val) {
if (val === null || (typeof val !== 'function' && typeof val !== 'object')) return val;
return JSON.parse(JSON.stringify(val));
}

export function DateTimeFormat(locale = undefined, options = undefined) {
// `= undefined` usage above seems unnecessary, but is actually needed because
// the spec requires Intl.DateTimeFormat.length === 0

// Clone inputs to protect against the caller later mutating `options` or
// `locale` before they're later used to (lazily) create built-in
// Intl.DateTimeFormat instances.
locale = deepClone(locale);
options = typeof options === 'undefined' ? {} : deepClone(options);

export function DateTimeFormat(locale = undefined, options = {}) {
if (!(this instanceof DateTimeFormat)) return new DateTimeFormat(locale, options);
const original = new IntlDateTimeFormat(locale, options);
const ro = original.resolvedOptions();
options = typeof options === 'undefined' ? {} : ObjectAssign({}, options);

this[TZ_GIVEN] = options.timeZone ? options.timeZone : null;
this[OPTIONS] = options;
Expand Down Expand Up @@ -353,7 +369,7 @@ function extractOverrides(temporalObj, main) {
const nanosecond = GetSlot(temporalObj, ISO_NANOSECOND);
const datetime = new DateTime(1970, 1, 1, hour, minute, second, millisecond, microsecond, nanosecond, main[CAL_ID]);
return {
instant: ES.BuiltinTimeZoneGetInstantFor(getPropLazy(main, TZ_RESOLVED), datetime, 'compatible'),
instant: ES.BuiltinTimeZoneGetInstantFor(getResolvedTimeZoneLazy(main), datetime, 'compatible'),
formatter: getPropLazy(main, TIME)
};
}
Expand All @@ -370,7 +386,7 @@ function extractOverrides(temporalObj, main) {
}
const datetime = new DateTime(isoYear, isoMonth, referenceISODay, 12, 0, 0, 0, 0, 0, calendar);
return {
instant: ES.BuiltinTimeZoneGetInstantFor(getPropLazy(main, TZ_RESOLVED), datetime, 'compatible'),
instant: ES.BuiltinTimeZoneGetInstantFor(getResolvedTimeZoneLazy(main), datetime, 'compatible'),
formatter: getPropLazy(main, YM)
};
}
Expand All @@ -387,7 +403,7 @@ function extractOverrides(temporalObj, main) {
}
const datetime = new DateTime(referenceISOYear, isoMonth, isoDay, 12, 0, 0, 0, 0, 0, calendar);
return {
instant: ES.BuiltinTimeZoneGetInstantFor(getPropLazy(main, TZ_RESOLVED), datetime, 'compatible'),
instant: ES.BuiltinTimeZoneGetInstantFor(getResolvedTimeZoneLazy(main), datetime, 'compatible'),
formatter: getPropLazy(main, MD)
};
}
Expand All @@ -402,7 +418,7 @@ function extractOverrides(temporalObj, main) {
}
const datetime = new DateTime(isoYear, isoMonth, isoDay, 12, 0, 0, 0, 0, 0, main[CAL_ID]);
return {
instant: ES.BuiltinTimeZoneGetInstantFor(getPropLazy(main, TZ_RESOLVED), datetime, 'compatible'),
instant: ES.BuiltinTimeZoneGetInstantFor(getResolvedTimeZoneLazy(main), datetime, 'compatible'),
formatter: getPropLazy(main, DATE)
};
}
Expand Down Expand Up @@ -439,7 +455,7 @@ function extractOverrides(temporalObj, main) {
);
}
return {
instant: ES.BuiltinTimeZoneGetInstantFor(getPropLazy(main, TZ_RESOLVED), datetime, 'compatible'),
instant: ES.BuiltinTimeZoneGetInstantFor(getResolvedTimeZoneLazy(main), datetime, 'compatible'),
formatter: getPropLazy(main, DATETIME)
};
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"browser": "dist/index.umd.js",
"types": "index.d.ts",
"scripts": {
"test": "node --no-warnings --experimental-modules --icu-data-dir node_modules/full-icu --loader ./test/resolve.source.mjs ./test/all.mjs",
"test": "time node --no-warnings --experimental-modules --icu-data-dir node_modules/full-icu --loader ./test/resolve.source.mjs ./test/all.mjs",
"build": "rollup -c rollup.config.js",
"prepublishOnly": "npm run build",
"playground": "node --experimental-modules --no-warnings --icu-data-dir node_modules/full-icu -r ./lib/init.js",
Expand Down
21 changes: 18 additions & 3 deletions test/intl.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1050,10 +1050,25 @@ describe('Intl', () => {
it('should return an Array', () => assert(Array.isArray(Intl.DateTimeFormat.supportedLocalesOf())));
});

const us = new Intl.DateTimeFormat('en-US', { timeZone: 'America/New_York' });
const at = new Intl.DateTimeFormat('de-AT', { timeZone: 'Europe/Vienna' });
// Verify that inputs to DateTimeFormat constructor are immune to mutation.
const optionsAT = {
timeZone: {
toString: () => 'Europe/Vienna',
toJSON: () => 'Europe/Vienna'
}
};
const optionsUS = { timeZone: 'America/New_York' };
const localesAT = ['de-AT'];
const us = new Intl.DateTimeFormat('en-US', optionsUS);
const at = new Intl.DateTimeFormat(localesAT, optionsAT);
optionsAT.timeZone = {
toString: () => 'Bogus/Time-Zone',
toJSON: () => 'Bogus/Time-Zone'
};
optionsUS.timeZone = 'Bogus/Time-Zone';
const us2 = new Intl.DateTimeFormat('en-US');
const at2 = new Intl.DateTimeFormat('de-AT');
const at2 = new Intl.DateTimeFormat(localesAT);
localesAT[0] = ['invalid locale'];
const usCalendar = us.resolvedOptions().calendar;
const atCalendar = at.resolvedOptions().calendar;
const t1 = '1976-11-18T14:23:30+00:00[UTC]';
Expand Down

0 comments on commit b268b7a

Please sign in to comment.