Skip to content

Fix installation of tns-core-modules during project creation #2790

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 9, 2017
Merged

Fix installation of tns-core-modules during project creation #2790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/npm-installation-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class NpmInstallationManager implements INpmInstallationManager {

packageName = packageName + (version ? `@${version}` : "");

let npmOptions: any = { silent: true };
let npmOptions: any = { silent: true, "save-exact": true };

if (dependencyType) {
npmOptions[dependencyType] = true;
Expand Down
11 changes: 7 additions & 4 deletions lib/services/project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export class ProjectService implements IProjectService {
private $projectHelper: IProjectHelper,
private $projectNameService: IProjectNameService,
private $projectTemplatesService: IProjectTemplatesService,
private $staticConfig: IStaticConfig) { }
private $staticConfig: IStaticConfig,
private $npmInstallationManager: INpmInstallationManager) { }

@exported("projectService")
public async createProject(projectOptions: IProjectSettings): Promise<void> {
Expand Down Expand Up @@ -50,10 +51,12 @@ export class ProjectService implements IProjectService {

await this.ensureAppResourcesExist(projectDir);

let packageName = constants.TNS_CORE_MODULES_NAME;
await this.$npm.install(packageName, projectDir, { save: true, "save-exact": true });

let templatePackageJsonData = this.getDataFromJson(templatePath);

if (!(templatePackageJsonData && templatePackageJsonData.dependencies && templatePackageJsonData.dependencies[constants.TNS_CORE_MODULES_NAME])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@rosen-vladimirov Isn't it more correct to install the correct version of the Template? I.e. if you install 3.0.0 template and you override the tns_core_modules, won't still there be a possibility that this 3.0.0 template doesn't work with them? For pure NS I don't know, but for NS+Angular I think it's likely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yyosifov the template versioning may differ and we also do not have control over the templates - basically everything is template.
Also the current code will install tns-core-modules only in case they are not defined in the template itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the Template contains the tns-core-modules-widgets package? Shall we add it to the list of specific dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the tns-core-modules-widgets are dependency of the tns-core-modules. In case they exist in the template, it means someone has a very good reason to add them. I suggest to skip checks for them until we find a proper use-case.

await this.$npmInstallationManager.install(constants.TNS_CORE_MODULES_NAME, projectDir, { dependencyType: "save" });
}

this.mergeProjectAndTemplateProperties(projectDir, templatePackageJsonData); //merging dependencies from template (dev && prod)
this.removeMergedDependencies(projectDir, templatePackageJsonData);

Expand Down
4 changes: 3 additions & 1 deletion test/project-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class ProjectIntegrationTest {
this.testInjector.register("fs", FileSystem);
this.testInjector.register("projectDataService", ProjectDataServiceLib.ProjectDataService);
this.testInjector.register("staticConfig", StaticConfig);
this.testInjector.register("analyticsService", { track: async () => undefined });
this.testInjector.register("analyticsService", { track: async (): Promise<any> => undefined });

this.testInjector.register("npmInstallationManager", NpmInstallationManager);
this.testInjector.register("npm", NpmLib.NodePackageManager);
Expand All @@ -130,6 +130,7 @@ class ProjectIntegrationTest {
return dummyString;
}
});
this.testInjector.register("npmInstallationManager", NpmInstallationManager);
}
}

Expand Down Expand Up @@ -471,6 +472,7 @@ describe("Project Service Tests", () => {
testInjector.register("projectTemplatesService", {});
testInjector.register("staticConfig", {});
testInjector.register("projectHelper", {});
testInjector.register("npmInstallationManager", {});

return testInjector;
};
Expand Down