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

Extensions that use original-fs will need to be modified to fall back to fs when running remote #188

Closed
Chuxel opened this issue May 6, 2019 · 6 comments
Labels
feature-request Request for new features or functionality

Comments

@Chuxel
Copy link
Member

Chuxel commented May 6, 2019

Electron provides a node module called original-fs that provides access to base implementation of the fs node module instead of Electron's tweaked version. (See here.)

The VS Code Server in VS Code Remote Development extensions uses a standard Node.js runtime instead of Electron, so extensions that use original-fs will fail in a remote environment because this alias to fs does not exist.

Extensions should stick with the base fs implementation when possible, but if original-fs is required when running inside VS Code, the code will need to fall back to fs if original-fs isn't found. The subversion extension (#135) is an example of one currently using it.

We need to doc this issue and required changes. As an enhancement, we could also shim original-fs in the remote environment to fs.

/cc: @rebornix @alexandrudima @egamma @kieferrm

@alexdima
Copy link
Member

alexdima commented May 7, 2019

👍 We could definitely shim original-fs to point to fs. Do we know how many extensions are impacted?

@Chuxel
Copy link
Member Author

Chuxel commented May 7, 2019

Not many. I did a search and only found two, but since this is likely a general class of problem, I doc’d that you should generally avoid electron dependencies unless you provide a fallback.

PR is here, see if you agree with the content: microsoft/vscode-docs#2632

The SVN author is going to patch. The issue was introduced in a recent change which is why we didn’t catch it pre release.

@rebornix
Copy link
Member

rebornix commented May 7, 2019

I found 3 extensions declare dependencies on original-fs

  • GoogleCloudTools.cloudcode
  • Shan.code-settings-sync - a ui extension
  • WakaTime.vscode-wakatime - a ui extension

@egamma
Copy link
Member

egamma commented May 7, 2019

Shan.code-settings-sync

We define this extension as a UI extension in the product.json so it should be OK.

@Chuxel Chuxel removed the doc label May 8, 2019
@Chuxel Chuxel removed their assignment May 8, 2019
@Chuxel
Copy link
Member Author

Chuxel commented May 8, 2019

Ok docs now reference the general recommendation of avoiding of relying on electron specific modules and that you should have a fallback if you do. SVN is looking at a fix, that would only leave two with an unknown number down the road. Probably not enough to jump on it urgently - should we leave it open for future consideration?

@egamma
Copy link
Member

egamma commented May 8, 2019

@Chuxel this issue has been analyzed and can be closed. I've filed a separate issue on shimming original-fs

@egamma egamma closed this as completed May 8, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants