Vscode: Instantiate native promise instead of WinJS.Promise

Created on 7 Nov 2018  路  12Comments  路  Source: microsoft/vscode

This is part of https://github.com/Microsoft/vscode/issues/53526 and points out files in which we call new WinJS.Promise. In addition to join, as, wrap, and wrapError that's how winjs promises end up in our system and we wanna get rid of them.

Please migrate to instantiating native promises with taking all care that's needed for this adoption.

@bpasero

  • [ ] src/vs/platform/lifecycle/electron-main/lifecycleMain.ts
debt engineering

Most helpful comment

Had to revert the one in lifecycleMain.ts, that one is a bit tricky, will revisit in December. But that is really the last one :hammer:

All 12 comments

Started this but need to wait until lower level stuff like src/vs/workbench/services/files/electron-browser/fileService.ts is converted.

Did:

src/vs/base/node/processes.ts
src/vs/workbench/parts/tasks/common/problemMatcher.ts
src/vs/workbench/parts/tasks/common/taskDefinitionRegistry.ts

@alexr00 can you do src/vs/workbench/parts/tasks/electron-browser/terminalTaskSystem.ts that I don't interfere with work you are doing.

Using Thenable helps when the file's dependencies aren't converted. e.g. in cases like

public foo(): Thenable<void> {
  return actuallyTPromise().then(() => something)
}

As long as you check that the code will be safe when the upstream method switches from TPromise to Promise.

Yes, there are three ways to deal with not-yet-converted dependencies (and I think that 1 and 2 are the best ways to tackle this)

  • use Thenable
  • or use Promise.resolve
  • use async-await

Makes sense. Since the fileService is on the list to be converted I will wait. Will ensure I only need to touch once.

Completed my set:

@joaomoreno

src/vs/base/common/async.ts
src/vs/base/common/paging.ts
src/vs/base/parts/tree/browser/treeModel.ts
src/vs/base/parts/tree/test/browser/treeModel.test.ts
src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts
src/vs/code/electron-main/sharedProcess.ts
src/vs/base/test/common/paging.test.ts
src/vs/platform/driver/electron-main/driver.ts
src/vs/platform/update/electron-main/updateService.win32.ts
src/vs/code/node/cli.ts

Words of warning for @Microsoft/vscode: since this touched both the old tree as well as async.ts, be on the lookout for bugs. Remember this issue.

ping @bpasero and @Tyriar

@bpasero Why the reassign? Are there more files I should look at?

I've done the files under my name.

@alexandrudima sorry I thought people would remove the ones that are done from the summary like Joao did.

Had to revert the one in lifecycleMain.ts, that one is a bit tricky, will revisit in December. But that is really the last one :hammer:

Ah whatever, converted the last one :100:

awesome!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sijad picture sijad  路  3Comments

philipgiuliani picture philipgiuliani  路  3Comments

lukehoban picture lukehoban  路  3Comments

shanalikhan picture shanalikhan  路  3Comments

mrkiley picture mrkiley  路  3Comments