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

[vscode] unzip node_modules for built-in extensions #5771

Merged
merged 1 commit into from
Jul 23, 2019
Merged

Conversation

akosyakov
Copy link
Member

What it does

fix #5756: unzip node_modules for built-in extensions

How to test

Review checklist

  • as an author, I have thoroughly tested my changes and carefully reviewed following the review guidelines

Reminder for reviewers

@@ -41,6 +42,13 @@ export class PluginVsCodeFileHandler implements PluginDeployerFileHandler {

const unpackedPath = path.resolve(this.unpackedFolder, path.basename(context.pluginEntry().path()));
await context.unzip(context.pluginEntry().path(), unpackedPath);
if (context.pluginEntry().path().endsWith('.tgz')) {
const extensionPath = path.join(unpackedPath, 'package');
const vscodeNodeModulesPath = path.join(extensionPath, 'vscode_node_modules.zip');
Copy link
Member Author

Choose a reason for hiding this comment

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

built-in extensions are already republished with zip for node modules: eclipse-theia/vscode-builtin-extensions@51e5e27#diff-1b65ea7e7a9dd6aa50436ecdd461f0aaR23

@benoitf
Copy link
Contributor

benoitf commented Jul 22, 2019

hello, why published file is not a vsix file ?

@akosyakov
Copy link
Member Author

akosyakov commented Jul 22, 2019

hello, why published file is not a vsix file ?

Because npm does not accept such formats? Or do you mean package everything into vsix then push it to npm and then unpackage twice in Theia first from npm then from vsix? We could it do like that as well if someone will do it today. I don't have time to look at it.

@AlexTugarev
Copy link
Contributor

testing right now...

@benoitf
Copy link
Contributor

benoitf commented Jul 22, 2019

@akosyakov yes I mean embedding the vsix file inside the tgz archive so at the end we just deploy vsix files, not a custom archive

Copy link
Contributor

@AlexTugarev AlexTugarev left a comment

Choose a reason for hiding this comment

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

Screen Shot 2019-07-22 at 13 15 47

The emmet extension is working nicely with this change!

@akosyakov
Copy link
Member Author

@benoitf Would it be fine to file an issue for it for now? There are probably would be gotchas since VS Code team does not package these extensions, some attributes or files could be missing and so on.

@akosyakov
Copy link
Member Author

If no one is going to propose a PR with an alternative approach, we are going to merge it tomorrow.

@akosyakov akosyakov added critical critical bugs / problems vscode issues related to VSCode compatibility labels Jul 22, 2019
@benoitf
Copy link
Contributor

benoitf commented Jul 22, 2019

I'm fine. Also maybe we should release these vsix on github release page instead of npmjs

@akosyakov
Copy link
Member Author

@benoitf an issues is here: eclipse-theia/vscode-builtin-extensions#5

Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
@akosyakov akosyakov merged commit ccaa5d8 into master Jul 23, 2019
@akosyakov akosyakov deleted the GH-5756 branch July 23, 2019 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
critical critical bugs / problems vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vscode emmet extension does not work anymore
3 participants