Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[on-hold]: Updated the vscode-uri version. #6712

Closed
wants to merge 2 commits into from
Closed

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Dec 6, 2019

on-hold: #4874 (comment)

To fix the dependency hoisting in the node_modules.

Signed-off-by: Akos Kitta kittaakos@typefox.io

What it does

It updates the vscode-uri version. Otherwise, the old, 1.x.x version is hoisted to the main node_modules folder.

yarn why vscode-uri
yarn why v1.19.1
[1/4] Why do we have the module "vscode-uri"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "vscode-uri@1.0.8"
info Has been hoisted to "vscode-uri"
info Reasons this module exists
   - "workspace-aggregator-071b277d-a6f4-4569-90f5-3d9422efaa2a" depends on it
   - Hoisted from "_project_#@theia#core#vscode-uri"
   - Hoisted from "_project_#@theia#task#vscode-uri"
   - Hoisted from "_project_#@theia#typescript#typescript-language-server#vscode-uri"
   - Hoisted from "_project_#@theia#languages#monaco-languageclient#vscode-uri"
info Disk size without dependencies: "104KB"
info Disk size with unique dependencies: "104KB"
info Disk size with transitive dependencies: "104KB"
info Number of shared dependencies: 0
=> Found "vscode-json-languageserver#vscode-uri@2.1.1"
info This module exists because "_project_#@theia#json#vscode-json-languageserver" depends on it.
info Disk size without dependencies: "104KB"
info Disk size with unique dependencies: "104KB"
info Disk size with transitive dependencies: "104KB"
info Number of shared dependencies: 0
=> Found "vscode-json-languageservice#vscode-uri@2.1.1"
info This module exists because "_project_#@theia#json#vscode-json-languageserver#vscode-json-languageservice" depends on it.
info Disk size without dependencies: "104KB"
info Disk size with unique dependencies: "104KB"
info Disk size with transitive dependencies: "104KB"
info Number of shared dependencies: 0
Done in 1.83s.
=> Found "vscode-uri@1.0.8"
info Has been hoisted to "vscode-uri"

How to test

Run yarn why you should see:

yarn why vscode-uri          
yarn why v1.19.1
[1/4] 🤔  Why do we have the module "vscode-uri"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "vscode-uri@2.1.1"
info Has been hoisted to "vscode-uri"
info Reasons this module exists
   - "workspace-aggregator-9bb122b1-d1b9-438f-8bca-64068c9cc66d" depends on it
   - Hoisted from "_project_#@theia#core#vscode-uri"
   - Hoisted from "_project_#@theia#task#vscode-uri"
   - Hoisted from "_project_#@theia#json#vscode-json-languageserver#vscode-uri"
   - Hoisted from "_project_#@theia#json#vscode-json-languageserver#vscode-json-languageservice#vscode-uri"
info Disk size without dependencies: "104KB"
info Disk size with unique dependencies: "104KB"
info Disk size with transitive dependencies: "104KB"
info Number of shared dependencies: 0
=> Found "typescript-language-server#vscode-uri@1.0.8"
info This module exists because "_project_#@theia#typescript#typescript-language-server" depends on it.
info Disk size without dependencies: "104KB"
info Disk size with unique dependencies: "104KB"
info Disk size with transitive dependencies: "104KB"
info Number of shared dependencies: 0
=> Found "monaco-languageclient#vscode-uri@1.0.8"
info This module exists because "_project_#@theia#languages#monaco-languageclient" depends on it.
info Disk size without dependencies: "104KB"
info Disk size with unique dependencies: "104KB"
info Disk size with transitive dependencies: "104KB"
info Number of shared dependencies: 0
✨  Done in 1.05s.
=> Found "vscode-uri@2.1.1"
info Has been hoisted to "vscode-uri"

Or just check the content of the ./node_modules/vscode-uri/package.json file before and after this PR change; you can see the difference.

Review checklist

Reminder for reviewers

Akos Kitta added 2 commits December 6, 2019 19:48
To fix the dependency hoisting in the `node_modules`.
Updated the imports between version `1.x.x` and `2.x.x`.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
 - This is the default in `vscode-uri@2.x.x`.
 - No need to disable the error throwing if the scheme is missing.
 - No/Invalid URI `scheme` falls back to `file`.

microsoft/vscode#66802 (comment)
Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@kittaakos kittaakos changed the title Updated the vscode-uri version. [on-hold]: Updated the vscode-uri version. Dec 6, 2019
Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, tests are green. Could not see regressions when running. Some extra formatting is included but I'm fine with it. Was the hoisting of this specific version causing issues? I know that as a rule of thumb it is nice to use the most up-to-date dependencies when possible.

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

Successfully merging this pull request may close these issues.

2 participants