Skip to content

Conversation

@rosen-vladimirov
Copy link
Contributor

Our npmInstallationManager is using the passed next string as a version and searches for directory called next in the npm cache.
Instead it should use the real version. Fix this by getting the result of npm call addToCache, which returns object with information about cached package.

With this change you can call:

tns platform add android@next

Fixes #1563

@rosen-vladimirov rosen-vladimirov added this to the 1.7.0 milestone Mar 4, 2016
@rosen-vladimirov rosen-vladimirov self-assigned this Mar 4, 2016
@ErjanGavalji
Copy link
Contributor

👍

// In some cases the package is not fully downloaded and there are missing directories
// Try removing the old package and add the real one to cache again
this.addCleanCopyToCache(packageName, version).wait();
cachedPackageData = this.addCleanCopyToCache(packageName, version).wait();
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the interface, addCleanCopyToCache(packageName: string, version: string): IFuture<void>;, so this reassignment of cachedPackageData is pointless.
I'm guessing the interface is a bit off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed :)

@Mitko-Kerezov
Copy link
Contributor

👍 after addressing comments

Our npmInstallationManager is using the passed `next` string as a version and searches for directory called `next` in the npm cache.
Instead it should use the real version. Fix this by getting the result of npm call addToCache, which returns object with information about cached package.
@rosen-vladimirov rosen-vladimirov force-pushed the vladimirov/fix-next-runtime branch from d18c984 to b75a4e4 Compare March 7, 2016 08:44
@ikoevska
Copy link
Contributor

ikoevska commented Mar 7, 2016

Are we documenting this somewhere?

@rosen-vladimirov
Copy link
Contributor Author

@ikoevska no, it should work this way. It's a bug that it didn't work :)

rosen-vladimirov added a commit that referenced this pull request Mar 7, 2016
@rosen-vladimirov rosen-vladimirov merged commit 91ce213 into master Mar 7, 2016
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/fix-next-runtime branch March 7, 2016 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants