Closed
Bug 885318
Opened 13 years ago
Closed 13 years ago
Name collision with Promise
Categories
(DevTools :: General, defect)
DevTools
General
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 25
People
(Reporter: baku, Assigned: past)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
|
147.60 KB,
patch
|
jwalker
:
review+
|
Details | Diff | Splinter Review |
devtools use a lot of Promise objects implemented in JS.
But bug 884279 renames Future to Promise and this makes a name collision.
Talking on IRC on #devtools, we found 2 possible solutions:
a) Rename Promise to JSPromise (or to something else)
b) do some destructuring assignment on import, like: let {resolve, reject, etc. } = Cu.import("promise.js");
Updated•13 years ago
|
Component: General → Developer Tools
Product: Toolkit → Firefox
Comment 2•13 years ago
|
||
In a separate thread we'd decided that we should be doing s/Promise/promise/g. Would this solve the problem?
| Reporter | ||
Comment 3•13 years ago
|
||
Yes, It does.
| Assignee | ||
Comment 5•13 years ago
|
||
This turned out to be a lerger patch than I expected, due to us using lowercase promise as a local variable identifier. Maybe destructuring on import would have resulted in a shorter patch, but I don't know for sure or care. All tests pass locally.
Attachment #773344 -
Flags: review?(jwalker)
| Assignee | ||
Comment 6•13 years ago
|
||
Comment 7•13 years ago
|
||
Comment on attachment 773344 [details] [diff] [review]
Rename Promise to promise to avoid collisions with the forthcoming DOM Promise implementation
Review of attachment 773344 [details] [diff] [review]:
-----------------------------------------------------------------
It's kind of annoying that we can't call 'promise' 'promise' any more. Ho Hum.
Attachment #773344 -
Flags: review?(jwalker) → review+
Comment 8•13 years ago
|
||
How about migrating to DOM Promise?
| Assignee | ||
Comment 9•13 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #8)
> How about migrating to DOM Promise?
This bug is a prerequisite for DOM Future to be renamed to Promise. We will likely migrate to that once its ready and has all the API methods we need.
| Assignee | ||
Comment 10•13 years ago
|
||
Whiteboard: [fixed-in-fx-team]
Comment 11•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 25
Updated•8 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•