Opened 2 years ago

Closed 15 months ago

#5747 closed change (rejected)

Start using promises in new options page

Reported by: saroyanm Assignee:
Priority: P3 Milestone:
Module: User-Interface Keywords:
Cc: greiner Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by saroyanm)

Background

We are making a lot of Async calls in the new options page using browser.runtime.sendMessage (Dont' mix with actual browser.runtime.sendMessage which return promise).
In order to make the code more modern and be able get use of Promises instead of regular Async calls we should update the Options Page accordingly, while all targeted browsers we plan to ship new options page support Promises.

What to change

Use Promises for Async calls in the desktop version of options page.

Change History (6)

comment:1 Changed 2 years ago by saroyanm

  • Description modified (diff)
  • Priority changed from Unknown to P4
  • Ready set

comment:2 follow-up: Changed 2 years ago by saroyanm

I'm not sure if the Priority is to low, do we want to ship this early rather than later ?

Sidenote: Ideally I think we would have adapted our message handler to use promises rather than wrapping each call individually ? Is there any reason we don't consider doing that ? So browser.runtime.sendMessage will return promise ?

comment:3 in reply to: ↑ 2 Changed 2 years ago by greiner

  • Priority changed from P4 to P3

Replying to saroyanm:

I'm not sure if the Priority is to low, do we want to ship this early rather than later ?

According to the definition of priorities, this should be given P3 because we definitely want this but it's not a priority.

Sidenote: Ideally I think we would have adapted our message handler to use promises rather than wrapping each call individually ? Is there any reason we don't consider doing that ? So browser.runtime.sendMessage will return promise ?

It's pretty much up to Platform which interfaces they want to provide us with. Let's not implement our own abstraction layer for the same APIs that Platform also uses. Otherwise we'd end up with a bunch of duplicated code.

comment:4 follow-up: Changed 16 months ago by greiner

Is this ticket still relevant or can we close it?

Because we've already started adopting promises in various places. Plus, not only does browser.runtime.sendMessage() return a promise but we have also started developing a convenience wrapper around it (see js/api.js).

comment:5 in reply to: ↑ 4 Changed 15 months ago by saroyanm

Replying to greiner:

Is this ticket still relevant or can we close it?

Because we've already started adopting promises in various places. Plus, not only does browser.runtime.sendMessage() return a promise but we have also started developing a convenience wrapper around it (see js/api.js).

I agree, this ticket is already irrelevant.

comment:6 Changed 15 months ago by saroyanm

  • Resolution set to rejected
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.