fix: pacote does not respect CLI's proxy configuration #3831
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
run ci |
lib/services/pacote-service.ts
Outdated
| return pacote.manifest(packageName, manifestOptions); | ||
| } | ||
|
|
||
| private async getPacoteBaseOptions(): Promise<IPacoteBaseOptions> { |
There was a problem hiding this comment.
this method should be below the public methods
lib/services/pacote-service.ts
Outdated
| public async extractPackage(packageName: string, destinationDirectory: string, options?: IPacoteExtractOptions): Promise<void> { | ||
| // strip: Remove the specified number of leading path elements. Pathnames with fewer elements will be silently skipped. More info: https://github.com/npm/node-tar/blob/e89c4d37519b1c20133a9f49d5f6b85fa34c203b/README.md | ||
| // C: Create an archive | ||
| this.$logger.trace(`Calling pacoteService.extractPackage for packageName: '${packageName}', destinationDir: '${destinationDirectory}' and options: ${options}`); | ||
| const extractOptions = { strip: 1, C: destinationDirectory }; | ||
| if (options) { | ||
| _.extend(extractOptions, options); | ||
| } | ||
|
|
||
| if (this.$fs.exists(packageName)) { |
There was a problem hiding this comment.
this logic is duplicated in both methods, extract it to a separate method
DimitarTachev
approved these changes
Aug 22, 2018
test/services/pacote-service.ts
Outdated
| tarballSourceStream.emit("error", new Error("my error")); | ||
| }); | ||
|
|
||
| await assert.isRejected(pacoteExtractPackagePromise, "my error"); |
There was a problem hiding this comment.
const expectedErrorMessage = "my error";
test/services/pacote-service.ts
Outdated
| tarExtractDestinationStream.emit("error", new Error("my error")); | ||
| }); | ||
|
|
||
| await assert.isRejected(pacoteExtractPackagePromise, "my error"); |
There was a problem hiding this comment.
const expectedErrorMessage = "my error";
Pacote does not respect the proxy settings set with `tns proxy set ...`. Pass the required options and add unit tests for the pacote service (use sinon instead of mocking with injector for the `pacote` and `tar` packages as the service is our wrapper for pacote).
rosen-vladimirov
force-pushed
the
vladimirov/fix-pacote-proxy
branch
from
63fd629 to
86c1557
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pacote does not respect the proxy settings set with
tns proxy set .... Pass the required options and add unit tests for the pacote service (use sinon instead of mocking with injector for thepacoteandtarpackages as the service is our wrapper for pacote).PR Checklist
What is the current behavior?
Unable to create project when working behind proxy.
What is the new behavior?
Project can be created when working behind proxy
Fixes #3824
NOTE: Merge after telerik/mobile-cli-lib#1131