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

Fix plugin loading to cope with modules that have immutable exports #5520

Merged
merged 1 commit into from
Jul 3, 2019
Merged

Conversation

sstone1
Copy link
Contributor

@sstone1 sstone1 commented Jun 19, 2019

Fixes #5514

Some node modules, such as ansi-styles, define the exports property in an immutable manner: https://github.com/chalk/ansi-styles/blob/master/index.js#L159

Because the exports property has no setter, it throws an error when we try to overwrite it. We can just delete it instead.

Also, you cannot reload native modules - an error is thrown when you attempt to do so:

Error: Failed to load /Users/sstone1/git/blockchain-vscode-extension/client/ibm-blockchain-platform-1.0.3/extension/node_modules/grpc/src/node/extension_binary/node-v64-darwin-x64-unknown/grpc_node.node. Module did not self-register.

... so I've also fixed the code to skip doing anything with native modules (which have an ID that ends with .node).

@sstone1 sstone1 requested review from benoitf and evidolob as code owners June 19, 2019 10:11
@akosyakov
Copy link
Member

@benoitf @evidolob please review when you have a chance

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

thanks @sstone1 👍

@sstone1
Copy link
Contributor Author

sstone1 commented Jul 2, 2019

@evidolob please could you review this PR?

@akosyakov
Copy link
Member

@sstone1 How can one verify it? Do you have an extension to test?

@akosyakov
Copy link
Member

@evidolob it would be nice if we get contributors do proper reviews (i.e. that everything from #5616 is covered and good review comments are provided) in addition of opening new PRs. Otherwise we are getting a lot of contributions but cannot really accept them.

@sstone1
Copy link
Contributor Author

sstone1 commented Jul 3, 2019

@akosyakov I've tested it locally with our IBM Blockchain Platform extension - here's the Che plugin configuration I've been using:

https://gist.githubusercontent.com/sstone1/2882d58638838914f3a4cabf67b50df1/raw/1c1e60d3299fffdb785b586afc530054cf5b024a/meta.yaml
https://github.com/sstone1/che-plugin-registry/releases/download/0.0.1/ibm-blockchain-platform-1.0.3.vsix

Our extension pulls in modules with immutable exports (ansi-styles) and native modules (grpc).

The behaviour I observed before the fix is that the extension loads once inside Che, and then when you F5 - it will not load again.

FYI - don't use the extension version from the VSCode marketplace - it won't work inside Che.

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

thanks for the fast response, I've verified with installing blockchain and go vscode extensions that they both can be loaded without regressions.

Screen Shot 2019-07-03 at 10 29 56

@akosyakov akosyakov merged commit 20d60b3 into eclipse-theia:master Jul 3, 2019
@akosyakov
Copy link
Member

Although i found #5626

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.

Cannot set property exports of #<Module> which has only a getter
3 participants