Skip to content

Ensure that Temporal prototypes aren't writable #108

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

Merged
merged 1 commit into from
Dec 14, 2021
Merged

Ensure that Temporal prototypes aren't writable #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 25 additions & 0 deletions lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,29 @@
import * as Temporal from './temporal';
import * as Intl from './intl';
import { toTemporalInstant } from './legacydate';

// Work around https://github.com/babel/babel/issues/2025.
Copy link
Contributor

Choose a reason for hiding this comment

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

This has already been fixed in Babel - maybe we should just update our build once they release instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know their release schedule? 7.16.4 was almost a month ago according to GitHub, so if it's monthly and we only have to wait a few days then that seems like a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually think it's somewhat unrelated to Babel's schedule, because (unless I'm missing something) anyone who uses babel while bundling their app will be vulnerable. So even if our bundling is OK with the new Babel, we'll still want to protect users who are using an older Babel version from being able to build code that replaces the prototype (which will break when they upgrade to the latest Babel).

BTW, we still need to merge the changes to Intl.DateTimeFormat prototype regardless of Babel.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we required at least 7.16.5 wouldn't users' babel update when they updated @js-temporal/polyfill to 0.3.0, as long as the versions were compatible?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://stackoverflow.com/questions/18875674/whats-the-difference-between-dependencies-devdependencies-and-peerdependencies

Maybe I'm mis-reading this, but it seems that devDependencies (which is what our dep on babel is) aren't installed transitively, so we wouldn't be forcing downstream users to update their babel versions.

That being said, I wonder if there's a way to "suggest" that update to users to encourage they update.... maybe an (ab)use of peerDependencies can do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, my understanding of build-time dependencies is that they're not enforced on the main app.

Regardless, I'm not sure it's worth the bother. We understand the problem and have a simple workaround that seems to work (and is a no-op at runtime if it's been transpiled with a fixed Babel). So I'm inclined to just declare success and move on. What do you guys think?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds reasonable - let's just merge this and move on :)

const types = [
Temporal.Instant,
Temporal.Calendar,
Temporal.PlainDate,
Temporal.PlainDateTime,
Temporal.Duration,
Temporal.PlainMonthDay,
// Temporal.Now, // plain object (not a constructor), so no `prototype`
Temporal.PlainTime,
Temporal.TimeZone,
Temporal.PlainYearMonth,
Temporal.ZonedDateTime
];
for (const type of types) {
const descriptor = Object.getOwnPropertyDescriptor(type, 'prototype') as PropertyDescriptor;
if (descriptor.configurable || descriptor.enumerable || descriptor.writable) {
descriptor.configurable = false;
descriptor.enumerable = false;
descriptor.writable = false;
Object.defineProperty(type, 'prototype', descriptor);
}
}

export { Temporal, Intl, toTemporalInstant };
2 changes: 1 addition & 1 deletion lib/init.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Object.defineProperty(globalThis, 'Temporal', {
enumerable: false,
configurable: true
});
const globalTemporal = (globalThis as any).Temporal;
const globalTemporal = (globalThis as unknown as { Temporal: typeof Temporal }).Temporal;
copy(globalTemporal, Temporal);
Object.defineProperty(globalTemporal, Symbol.toStringTag, {
value: 'Temporal',
Expand Down
11 changes: 9 additions & 2 deletions lib/intl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,6 @@ Object.defineProperty(DateTimeFormatImpl, 'name', {
value: 'DateTimeFormat'
});

export const DateTimeFormat = DateTimeFormatImpl as unknown as typeof Intl.DateTimeFormat;

DateTimeFormatImpl.supportedLocalesOf = function (
locales: Params['supportedLocalesOf'][0],
options: Params['supportedLocalesOf'][1]
Expand All @@ -199,6 +197,15 @@ if ('formatRangeToParts' in IntlDateTimeFormat.prototype) {

DateTimeFormatImpl.prototype = Object.create(IntlDateTimeFormat.prototype, properties);

// Ensure that the prototype isn't writeable.
Object.defineProperty(DateTimeFormatImpl, 'prototype', {
writable: false,
enumerable: false,
configurable: false
});

export const DateTimeFormat = DateTimeFormatImpl as unknown as typeof Intl.DateTimeFormat;

function resolvedOptions(this: DateTimeFormatImpl): Return['resolvedOptions'] {
return this[ORIGINAL].resolvedOptions();
}
Expand Down