Closed Bug 1260938 Opened 8 years ago Closed 8 years ago

Add the `ThreadSafeDevToolsUtils.isSet` utility

Categories

(DevTools :: General, defect)

defect
Not set
normal

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Assignee: nobody → nfitzgerald
Blocks: 1249788, 1260939
Status: NEW → ASSIGNED
Comment on attachment 8736522 [details] [diff] [review]
Add the `ThreadSafeDevToolsUtils.isSet` utility

Review of attachment 8736522 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/shared/ThreadSafeDevToolsUtils.js
@@ +248,5 @@
> + * Return true iff `thing` is a `Set` object (possibly from another global).
> + */
> +exports.isSet = function(thing) {
> +  return Object.prototype.toString.call(thing) === "[object Set]";
> +};

This is a pretty ugly technique. I see that we've already got other predicates like this, though.

What would the truly ES6-esque way of doing this be?

::: devtools/shared/tests/unit/test_isSet.js
@@ +7,5 @@
> +function run_test() {
> +  const { isSet } = DevToolsUtils;
> +
> +  equal(isSet(new Set()), true);
> +  equal(isSet(new Map()), false);

Should we test it on primitives and plain objects too?
Attachment #8736522 - Flags: review?(jimb) → review+
(In reply to Jim Blandy :jimb from comment #3)
> Comment on attachment 8736522 [details] [diff] [review]
> Add the `ThreadSafeDevToolsUtils.isSet` utility
> 
> Review of attachment 8736522 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/shared/ThreadSafeDevToolsUtils.js
> @@ +248,5 @@
> > + * Return true iff `thing` is a `Set` object (possibly from another global).
> > + */
> > +exports.isSet = function(thing) {
> > +  return Object.prototype.toString.call(thing) === "[object Set]";
> > +};
> 
> This is a pretty ugly technique. I see that we've already got other
> predicates like this, though.
> 
> What would the truly ES6-esque way of doing this be?

Asked around, no better answers. Suggestion was to add a chrome-only instrinsic, which seems like overkill here. Apparently ES6 is going to make this even worse by adding a hook for any old class/object to customize <foo> in "[object <foo>]"... Yay.

> ::: devtools/shared/tests/unit/test_isSet.js
> @@ +7,5 @@
> > +function run_test() {
> > +  const { isSet } = DevToolsUtils;
> > +
> > +  equal(isSet(new Set()), true);
> > +  equal(isSet(new Map()), false);
> 
> Should we test it on primitives and plain objects too?

Sure, I'll add that.
Attachment #8736522 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/49adb658e099
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.