Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Split module into @types/vscode and vscode-test #147

Closed
octref opened this issue Mar 1, 2019 · 8 comments
Closed

Split module into @types/vscode and vscode-test #147

octref opened this issue Mar 1, 2019 · 8 comments
Assignees

Comments

@octref
Copy link
Contributor

octref commented Mar 1, 2019

Per the recent incidents (microsoft/vscode#69656), I think it's a good opportunity for us to split vscode node modules into 2 pieces:

  • @types/vscode
  • vscode-test

Explanation

vscode node module contains 3 scripts:

  • compile: no longer used
  • install: used to fetch vscode.d.ts according to engine.vscode in package.json
  • test: used for extension test (also lib/testRunner)

Problems

  • install asks update server for finding matching vscode.d.ts version (actual dts files fetched from GitHub). When update service is down, all extension deps install (CI or local development) could fail.
  • yarn install and npm install can't be completed offline, even if they have caching: Add Switch for Offline Installation #93
  • test depends on a lot of node modules (13MB). Everyone has to download them even if they just want to use the API.
  • test needs some improvements. For example, for running extension tests against multiple workspaces, I had to pull out the test scripts and modify them manually Vetur. Also add flag / env var to enable downloading and unzipping to .vscode-test without running tests... #124, people want functionalities exposed from test so they can build upon them.

Benefits of splitting vscode into @types/vscode and vscode-test

  • No awkward postinstall scripts after each npm install for extension
  • Users don't have to pull in the 11MB dependencies of test. They pull a few kb of vscode.d.ts from @types/vscode, and it's fully cached by npm/yarn.
  • vscode.d.ts are hosted on NPM which is pretty reliable. d.ts live in a @types package as they should be.
  • test now lives in its own repo. It's easier to make improvements, release new versions, expose functionalities (for example, import { downloadVSCodeExecutable } from vscode-test).

Challenges

We can't put all versions of vscode.d.ts into @types/vscode and let TS infer which one, from engines.vscode.

However I think we can publish vscode.d.ts the same version as we do for VS Code, since all our 1.x API are backwards compatible. Most users also auto-update to latest version, so extension authors could use "@types/vscode": "^1.31.0" and safely get minor updates.

/cc @Microsoft/vscode for thoughts.

@alexdima
Copy link
Member

alexdima commented Mar 1, 2019

👍 I like the proposal, I would suggest the small twist of shipping the .d.ts file directly within the vscode npm module instead of using @types/vscode. But that means we have to remember to always publish a new vscode npm module when we create a stable release. That should be fine, given we do it strictly after a VS Code release.

I would also vote for rewriting the script which downloads a vscode release to use https directly and the native installed tar instead of 13MB of gulp-* dependencies.

@octref
Copy link
Contributor Author

octref commented Mar 2, 2019

rewriting the script which downloads a vscode release to use https directly

Agreed, something like this already works for macOS/Linux, which is what I'm using on Travis. vscode-test should also contain minimal or zero dependencies, so we'd be exempted from issues like event-stream.

let downloadPlatform

switch (process.platform) {
  case 'darwin':
    downloadPlatform = 'darwin'
    break
  case 'win32':
    downloadPlatform = 'win32-archive'
    break
  default:
    downloadPlatform = 'linux-x64'
}

const url = `https://update.code.visualstudio.com/latest/${downloadPlatform}/stable`

const https = require('https')
const fs = require('fs')
const cp = require('child_process')

https.get(url, res => {
  const zipUrl = res.headers.location

  if (zipUrl.endsWith('.zip')) {
    const outStream = fs.createWriteStream('vscode.zip')
    https.get(zipUrl, res => {
      res.pipe(outStream)
    })
    outStream.on('close', () => {
      if (process.platform === 'win32') {
        // need some windows powershell script or a zip library that supports windows
      } else {
        cp.execSync('unzip vscode.zip')
      }
    })
  } else {
    const outStream = fs.createWriteStream('vscode.tgz')
    https.get(zipUrl, res => {
      res.pipe(outStream)
    })
    outStream.on('close', () => {
      cp.execSync('tar -xzf vscode.tgz')
    })
  }
})

@bpasero bpasero transferred this issue from microsoft/vscode Mar 2, 2019
@bpasero bpasero self-assigned this Mar 2, 2019
@bpasero bpasero changed the title Split vscode NPM node module into @types/vscode and vscode-test Split module into @types/vscode and vscode-test Mar 2, 2019
@bpasero
Copy link
Member

bpasero commented Mar 2, 2019

@octref wanna do some PRs on this? E.g. the download part?

@octref
Copy link
Contributor Author

octref commented Mar 2, 2019

@bpasero Sure.

@octref
Copy link
Contributor Author

octref commented Mar 25, 2019

But that means we have to remember to always publish a new vscode npm module when we create a stable release.

In the end I chose @types/vscode since it could have easy-to-follow versioning (@types/vscode 1.X corresponds to VS Code 1.X release). With vscode it would be a breaking change and the module would be 2.X.

More work are captured in microsoft/vscode#71048. I'll get back to it in May after I wrap up my work in Vetur.

@octref octref closed this as completed Mar 25, 2019
@bpasero
Copy link
Member

bpasero commented Mar 25, 2019

@octref would it make sense to use https://github.com/Microsoft/node-request-light ?

//cc @aeschli

@octref
Copy link
Contributor Author

octref commented Mar 25, 2019

I picked those dependencies because we use them in the product, too: https://github.com/Microsoft/vscode/blob/81a7dff142ee901daf6e17c2b7b1d6391c634717/package.json#L32-L33

@bpasero
Copy link
Member

bpasero commented Mar 25, 2019

@octref ok got it, I somehow thought someone in the team mentioned during standup that there is another option for dependencies.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants