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.

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

ToDateTimeOptions should avoid making an observable internal-use object #706

Closed
gibson042 opened this issue Aug 24, 2022 · 2 comments
Closed
Labels
c: datetime Component: dates, times, timezones s: help wanted Status: help wanted; needs proposal champion
Milestone

Comments

@gibson042
Copy link
Contributor

(created from #493 (comment) )

ToDateTimeOptions creates an object inheriting from provided options and performs some observable writes upon it, which is unnecessarily weird.

/* Make options a proxy that spies on lots of operations until disabled. */
let spy = {};
const wrapperLabels = new Map();
const options = new Proxy({ignored: true}, {
  ...Object.fromEntries(
    ["getOwnPropertyDescriptor", "defineProperty", "has", "get", "ownKeys"].map(trapName => {
      spy[trapName] = new Map();
      return [trapName, (...args) => {
        const result = Reflect[trapName](...args);
        if (spy) {
          const m = spy[trapName];
          const propKey = args[1];
          const n = (m.get(propKey) || 0) + 1;
          m.set(propKey, n);
          let details = args.slice(1);
          /* Use the get trap to discover system-created objects. */
          if (trapName === "get") {
            const receiver = args[2];
            let receiverLabel;
            if (receiver === options) {
              receiverLabel = "options";
            } else if (Object.getPrototypeOf(receiver) === options) {
              if (!wrapperLabels.has(receiver)) {
                wrapperLabels.set(receiver, "wrapper" + wrapperLabels.size);
              }
              receiverLabel = wrapperLabels.get(receiver);
            } else {
              receiverLabel = "unknown receiver " + String(receiver);
            }
            details = ["@" + receiverLabel, propKey];
          }
          print(n, trapName, ...details, "=>", result);
        }
        return result;
      }];
    })
  )
});
new Intl.DateTimeFormat(undefined, options);
spy = null;
for (const [obj, label] of new Map([[options, "options"], ...wrapperLabels.entries()])) {
  print(`${label} = {\n${
    Reflect.ownKeys(obj).map(k => `\t${String(k)}: ${String(obj[k])},\n`).join("")
  }}`);
}
$ eshost -s demo.js
#### GraalJS, JavaScriptCore, SpiderMonkey, V8
1 get @wrapper0 weekday => undefined
1 get @wrapper0 year => undefined
1 get @wrapper0 month => undefined
1 get @wrapper0 day => undefined
1 get @wrapper0 dayPeriod => undefined
1 get @wrapper0 hour => undefined
1 get @wrapper0 minute => undefined
1 get @wrapper0 second => undefined
1 get @wrapper0 fractionalSecondDigits => undefined
1 get @wrapper0 dateStyle => undefined
1 get @wrapper0 timeStyle => undefined
1 get @wrapper0 localeMatcher => undefined
1 get @wrapper0 calendar => undefined
1 get @wrapper0 numberingSystem => undefined
1 get @wrapper0 hour12 => undefined
1 get @wrapper0 hourCycle => undefined
1 get @wrapper0 timeZone => undefined
2 get @wrapper0 weekday => undefined
1 get @wrapper0 era => undefined
2 get @wrapper0 dayPeriod => undefined
2 get @wrapper0 hour => undefined
2 get @wrapper0 minute => undefined
2 get @wrapper0 second => undefined
2 get @wrapper0 fractionalSecondDigits => undefined
1 get @wrapper0 timeZoneName => undefined
1 get @wrapper0 formatMatcher => undefined
2 get @wrapper0 dateStyle => undefined
2 get @wrapper0 timeStyle => undefined
options = {
	ignored: true,
}
wrapper0 = {
	year: numeric,
	month: numeric,
	day: numeric,
}
@anba
Copy link
Contributor

anba commented Aug 24, 2022

#237 (comment) has a link to patch which avoids creating an intermediate object.

@sffc sffc added this to the ES 2023 milestone Aug 25, 2022
@sffc sffc added s: help wanted Status: help wanted; needs proposal champion c: datetime Component: dates, times, timezones labels Aug 25, 2022
anba added a commit to anba/ecma402 that referenced this issue Sep 20, 2022
…mat objects

This commits inlines the default values processing and conflicting
date-time styles checks from `ToDateTimeOptions` into
`CreateDateTimeFormat`.

Fixes tc39#237
Fixes tc39#706
anba added a commit to anba/ecma402 that referenced this issue Jan 12, 2023
…mat objects

This commits inlines the default values processing and conflicting
date-time styles checks from `ToDateTimeOptions` into
`CreateDateTimeFormat`.

Fixes tc39#237
Fixes tc39#706
anba added a commit to anba/ecma402 that referenced this issue Apr 5, 2023
…mat objects

This commits inlines the default values processing and conflicting
date-time styles checks from `ToDateTimeOptions` into
`CreateDateTimeFormat`.

Fixes tc39#237
Fixes tc39#706
FrankYFTang pushed a commit to FrankYFTang/ecma402 that referenced this issue Jul 26, 2023
…mat objects

This commits inlines the default values processing and conflicting
date-time styles checks from `ToDateTimeOptions` into
`CreateDateTimeFormat`.

Fixes tc39#237
Fixes tc39#706
@anba
Copy link
Contributor

anba commented Aug 4, 2023

Fixed in #709.

@anba anba closed this as completed Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: datetime Component: dates, times, timezones s: help wanted Status: help wanted; needs proposal champion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants