Skip to content

Commit edd0fae

Browse files
FatmeFatme
authored
Merge pull request #4671 from NativeScript/fatme/project-changes
fix: don't check all node_modules from `checkForChanges` method
2 parents c198956 + ddf89d8 commit edd0fae

File tree

7 files changed

+47
-103
lines changed

7 files changed

+47
-103
lines changed

‎lib/controllers/prepare-controller.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { hook } from "../common/helpers";
44
import { performanceLog } from "../common/decorators";
55
import { EventEmitter } from "events";
66
import * as path from "path";
7-
import { PREPARE_READY_EVENT_NAME, WEBPACK_COMPILATION_COMPLETE } from "../constants";
7+
import { PREPARE_READY_EVENT_NAME, WEBPACK_COMPILATION_COMPLETE, PACKAGE_JSON_FILE_NAME } from "../constants";
88

99
interface IPlatformWatcherData {
1010
webpackCompilerProcess: child_process.ChildProcess;
@@ -115,6 +115,8 @@ export class PrepareController extends EventEmitter {
115115
}
116116

117117
const patterns = [
118+
path.join(projectData.projectDir, PACKAGE_JSON_FILE_NAME),
119+
path.join(projectData.getAppDirectoryPath(), PACKAGE_JSON_FILE_NAME),
118120
path.join(projectData.getAppResourcesRelativeDirectoryPath(), platformData.normalizedPlatformName),
119121
`node_modules/**/platforms/${platformData.platformNameLowerCase}/`
120122
];
@@ -130,7 +132,7 @@ export class PrepareController extends EventEmitter {
130132
const watcher = choki.watch(patterns, watcherOptions)
131133
.on("all", async (event: string, filePath: string) => {
132134
filePath = path.join(projectData.projectDir, filePath);
133-
this.$logger.info(`Chokidar raised event ${event} for ${filePath}.`);
135+
this.$logger.trace(`Chokidar raised event ${event} for ${filePath}.`);
134136
this.emitPrepareEvent({ files: [], hmrData: null, hasNativeChanges: true, platform: platformData.platformNameLowerCase });
135137
});
136138

‎lib/controllers/run-controller.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,9 @@ export class RunController extends EventEmitter implements IRunController {
302302

303303
try {
304304
if (data.hasNativeChanges) {
305-
await this.$prepareNativePlatformService.prepareNativePlatform(platformData, projectData, prepareData);
306-
await deviceDescriptor.buildAction();
305+
if (await this.$prepareNativePlatformService.prepareNativePlatform(platformData, projectData, prepareData)) {
306+
await deviceDescriptor.buildAction();
307+
}
307308
}
308309

309310
const isInHMRMode = liveSyncInfo.useHotModuleReload && data.hmrData && data.hmrData.hash;

‎lib/definitions/project-changes.d.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ interface IPrepareInfo extends IAddedNativePlatform, IAppFilesHashes {
1414

1515
interface IProjectChangesInfo extends IAddedNativePlatform {
1616
appResourcesChanged: boolean;
17-
modulesChanged: boolean;
1817
configChanged: boolean;
19-
packageChanged: boolean;
2018
nativeChanged: boolean;
2119
signingChanged: boolean;
2220

‎lib/services/platform/prepare-native-platform-service.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ export class PrepareNativePlatformService implements IPrepareNativePlatformServi
2121

2222
const changesInfo = await this.$projectChangesService.checkForChanges(platformData, projectData, prepareData);
2323

24-
const hasModulesChange = !changesInfo || changesInfo.modulesChanged;
24+
const hasNativeModulesChange = !changesInfo || changesInfo.nativeChanged;
2525
const hasConfigChange = !changesInfo || changesInfo.configChanged;
2626
const hasChangesRequirePrepare = !changesInfo || changesInfo.changesRequirePrepare;
2727

28-
const hasChanges = hasModulesChange || hasConfigChange || hasChangesRequirePrepare;
28+
const hasChanges = hasNativeModulesChange || hasConfigChange || hasChangesRequirePrepare;
2929

3030
if (changesInfo.hasChanges) {
3131
await this.cleanProject(platformData, { release });
@@ -37,11 +37,11 @@ export class PrepareNativePlatformService implements IPrepareNativePlatformServi
3737
await platformData.platformProjectService.prepareProject(projectData, prepareData);
3838
}
3939

40-
if (hasModulesChange) {
40+
if (hasNativeModulesChange) {
4141
await this.$nodeModulesBuilder.prepareNodeModules(platformData, projectData);
4242
}
4343

44-
if (hasModulesChange || hasConfigChange) {
44+
if (hasNativeModulesChange || hasConfigChange) {
4545
await platformData.platformProjectService.processConfigurationFilesFromAppResources(projectData, { release });
4646
await platformData.platformProjectService.handleNativeDependenciesChange(projectData, { release });
4747
}

‎lib/services/project-changes-service.ts

Lines changed: 31 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as path from "path";
2-
import { NODE_MODULES_FOLDER_NAME, NativePlatformStatus, PACKAGE_JSON_FILE_NAME, APP_GRADLE_FILE_NAME, BUILD_XCCONFIG_FILE_NAME } from "../constants";
2+
import { NativePlatformStatus, PACKAGE_JSON_FILE_NAME, APP_GRADLE_FILE_NAME, BUILD_XCCONFIG_FILE_NAME, PLATFORMS_DIR_NAME } from "../constants";
33
import { getHash, hook } from "../common/helpers";
44
import { PrepareData } from "../data/prepare-data";
55

@@ -8,24 +8,20 @@ const prepareInfoFileName = ".nsprepareinfo";
88
class ProjectChangesInfo implements IProjectChangesInfo {
99

1010
public appResourcesChanged: boolean;
11-
public modulesChanged: boolean;
1211
public configChanged: boolean;
13-
public packageChanged: boolean;
1412
public nativeChanged: boolean;
1513
public signingChanged: boolean;
1614
public nativePlatformStatus: NativePlatformStatus;
1715

1816
public get hasChanges(): boolean {
19-
return this.packageChanged ||
17+
return this.nativeChanged ||
2018
this.appResourcesChanged ||
21-
this.modulesChanged ||
2219
this.configChanged ||
2320
this.signingChanged;
2421
}
2522

2623
public get changesRequireBuild(): boolean {
27-
return this.packageChanged ||
28-
this.appResourcesChanged ||
24+
return this.appResourcesChanged ||
2925
this.nativeChanged;
3026
}
3127

@@ -39,15 +35,15 @@ export class ProjectChangesService implements IProjectChangesService {
3935

4036
private _changesInfo: IProjectChangesInfo;
4137
private _prepareInfo: IPrepareInfo;
42-
private _newFiles: number = 0;
4338
private _outputProjectMtime: number;
4439
private _outputProjectCTime: number;
4540

4641
constructor(
4742
private $devicePlatformsConstants: Mobile.IDevicePlatformsConstants,
4843
private $fs: IFileSystem,
4944
private $logger: ILogger,
50-
public $hooksService: IHooksService) {
45+
public $hooksService: IHooksService,
46+
private $nodeModulesDependenciesBuilder: INodeModulesDependenciesBuilder) {
5147
}
5248

5349
public get currentChanges(): IProjectChangesInfo {
@@ -59,26 +55,24 @@ export class ProjectChangesService implements IProjectChangesService {
5955
this._changesInfo = new ProjectChangesInfo();
6056
const isNewPrepareInfo = await this.ensurePrepareInfo(platformData, projectData, prepareData);
6157
if (!isNewPrepareInfo) {
62-
this._newFiles = 0;
63-
64-
this._changesInfo.packageChanged = this.isProjectFileChanged(projectData.projectDir, platformData);
65-
6658
const platformResourcesDir = path.join(projectData.appResourcesDirectoryPath, platformData.normalizedPlatformName);
67-
this._changesInfo.appResourcesChanged = this.containsNewerFiles(platformResourcesDir, null, projectData);
68-
/*done because currently all node_modules are traversed, a possible improvement could be traversing only the production dependencies*/
69-
this._changesInfo.nativeChanged = this.containsNewerFiles(
70-
path.join(projectData.projectDir, NODE_MODULES_FOLDER_NAME),
71-
path.join(projectData.projectDir, NODE_MODULES_FOLDER_NAME, "tns-ios-inspector"),
72-
projectData,
73-
this.fileChangeRequiresBuild);
59+
this._changesInfo.appResourcesChanged = this.containsNewerFiles(platformResourcesDir, projectData);
60+
61+
this.$nodeModulesDependenciesBuilder.getProductionDependencies(projectData.projectDir)
62+
.filter(dep => dep.nativescript && this.$fs.exists(path.join(dep.directory, PLATFORMS_DIR_NAME, platformData.platformNameLowerCase)))
63+
.forEach(dep => {
64+
this._changesInfo.nativeChanged = this._changesInfo.nativeChanged ||
65+
this.containsNewerFiles(path.join(dep.directory, PLATFORMS_DIR_NAME, platformData.platformNameLowerCase), projectData) ||
66+
this.isFileModified(path.join(dep.directory, PACKAGE_JSON_FILE_NAME));
67+
});
68+
69+
if (!this._changesInfo.nativeChanged) {
70+
this._prepareInfo.projectFileHash = this.getProjectFileStrippedHash(projectData.projectDir, platformData);
71+
this._changesInfo.nativeChanged = this.isProjectFileChanged(projectData.projectDir, platformData);
72+
}
7473

7574
this.$logger.trace(`Set nativeChanged to ${this._changesInfo.nativeChanged}.`);
7675

77-
if (this._newFiles > 0 || this._changesInfo.nativeChanged) {
78-
this.$logger.trace(`Setting modulesChanged to true, newFiles: ${this._newFiles}, nativeChanged: ${this._changesInfo.nativeChanged}`);
79-
this._changesInfo.modulesChanged = true;
80-
}
81-
8276
if (platformData.platformNameLowerCase === this.$devicePlatformsConstants.iOS.toLowerCase()) {
8377
this._changesInfo.configChanged = this.filesChanged([path.join(platformResourcesDir, platformData.configurationFileName),
8478
path.join(platformResourcesDir, "LaunchScreen.storyboard"),
@@ -101,16 +95,11 @@ export class ProjectChangesService implements IProjectChangesService {
10195
if (prepareData.release !== this._prepareInfo.release) {
10296
this.$logger.trace(`Setting all setting to true. Current options are: `, prepareData, " old prepare info is: ", this._prepareInfo);
10397
this._changesInfo.appResourcesChanged = true;
104-
this._changesInfo.modulesChanged = true;
10598
this._changesInfo.configChanged = true;
10699
this._prepareInfo.release = prepareData.release;
107100
}
108-
if (this._changesInfo.packageChanged) {
109-
this.$logger.trace("Set modulesChanged to true as packageChanged is true");
110-
this._changesInfo.modulesChanged = true;
111-
}
112-
if (this._changesInfo.modulesChanged || this._changesInfo.appResourcesChanged) {
113-
this.$logger.trace(`Set configChanged to true, current value of moduleChanged is: ${this._changesInfo.modulesChanged}, appResourcesChanged is: ${this._changesInfo.appResourcesChanged}`);
101+
if (this._changesInfo.appResourcesChanged) {
102+
this.$logger.trace(`Set configChanged to true, appResourcesChanged is: ${this._changesInfo.appResourcesChanged}`);
114103
this._changesInfo.configChanged = true;
115104
}
116105
if (this._changesInfo.hasChanges) {
@@ -119,8 +108,6 @@ export class ProjectChangesService implements IProjectChangesService {
119108
if (this._prepareInfo.changesRequireBuild) {
120109
this._prepareInfo.changesRequireBuildTime = this._prepareInfo.time;
121110
}
122-
123-
this._prepareInfo.projectFileHash = this.getProjectFileStrippedHash(projectData.projectDir, platformData);
124111
}
125112

126113
this._changesInfo.nativePlatformStatus = this._prepareInfo.nativePlatformStatus;
@@ -191,7 +178,6 @@ export class ProjectChangesService implements IProjectChangesService {
191178
this._outputProjectCTime = 0;
192179
this._changesInfo = this._changesInfo || new ProjectChangesInfo();
193180
this._changesInfo.appResourcesChanged = true;
194-
this._changesInfo.modulesChanged = true;
195181
this._changesInfo.configChanged = true;
196182
return true;
197183
}
@@ -229,48 +215,33 @@ export class ProjectChangesService implements IProjectChangesService {
229215
return false;
230216
}
231217

232-
private containsNewerFiles(dir: string, skipDir: string, projectData: IProjectData, processFunc?: (filePath: string, projectData: IProjectData) => boolean): boolean {
218+
private containsNewerFiles(dir: string, projectData: IProjectData): boolean {
233219
const dirName = path.basename(dir);
234220
this.$logger.trace(`containsNewerFiles will check ${dir}`);
235221
if (_.startsWith(dirName, '.')) {
236222
this.$logger.trace(`containsNewerFiles returns false for ${dir} as its name starts with dot (.) .`);
237223
return false;
238224
}
239225

240-
const dirFileStat = this.$fs.getFsStats(dir);
241-
if (this.isFileModified(dirFileStat, dir)) {
226+
if (this.isFileModified(dir)) {
242227
this.$logger.trace(`containsNewerFiles returns true for ${dir} as the dir itself has been modified.`);
243228
return true;
244229
}
245230

246231
const files = this.$fs.readDirectory(dir);
247232
for (const file of files) {
248233
const filePath = path.join(dir, file);
249-
if (filePath === skipDir) {
250-
continue;
251-
}
252234

253235
const fileStats = this.$fs.getFsStats(filePath);
254-
const changed = this.isFileModified(fileStats, filePath);
236+
const changed = this.isFileModified(filePath, fileStats);
255237

256238
if (changed) {
257-
this.$logger.trace(`File ${filePath} has been changed.`);
258-
if (processFunc) {
259-
this._newFiles++;
260-
this.$logger.trace(`Incremented the newFiles counter. Current value is ${this._newFiles}`);
261-
const filePathRelative = path.relative(projectData.projectDir, filePath);
262-
if (processFunc.call(this, filePathRelative, projectData)) {
263-
this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`);
264-
return true;
265-
}
266-
} else {
267-
this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`);
268-
return true;
269-
}
239+
this.$logger.trace(`containsNewerFiles returns true for ${dir}. The modified file is ${filePath}`);
240+
return true;
270241
}
271242

272243
if (fileStats.isDirectory()) {
273-
if (this.containsNewerFiles(filePath, skipDir, projectData, processFunc)) {
244+
if (this.containsNewerFiles(filePath, projectData)) {
274245
this.$logger.trace(`containsNewerFiles returns true for ${dir}.`);
275246
return true;
276247
}
@@ -281,9 +252,10 @@ export class ProjectChangesService implements IProjectChangesService {
281252
return false;
282253
}
283254

284-
private isFileModified(filePathStat: IFsStats, filePath: string): boolean {
285-
let changed = filePathStat.mtime.getTime() >= this._outputProjectMtime ||
286-
filePathStat.ctime.getTime() >= this._outputProjectCTime;
255+
private isFileModified(filePath: string, filePathStats?: IFsStats): boolean {
256+
filePathStats = filePathStats || this.$fs.getFsStats(filePath);
257+
let changed = filePathStats.mtime.getTime() >= this._outputProjectMtime ||
258+
filePathStats.ctime.getTime() >= this._outputProjectCTime;
287259

288260
if (!changed) {
289261
const lFileStats = this.$fs.getLsStats(filePath);
@@ -293,29 +265,5 @@ export class ProjectChangesService implements IProjectChangesService {
293265

294266
return changed;
295267
}
296-
297-
private fileChangeRequiresBuild(file: string, projectData: IProjectData) {
298-
if (path.basename(file) === PACKAGE_JSON_FILE_NAME) {
299-
return true;
300-
}
301-
const projectDir = projectData.projectDir;
302-
if (_.startsWith(path.join(projectDir, file), projectData.appResourcesDirectoryPath)) {
303-
return true;
304-
}
305-
if (_.startsWith(file, NODE_MODULES_FOLDER_NAME)) {
306-
let filePath = file;
307-
while (filePath !== NODE_MODULES_FOLDER_NAME) {
308-
filePath = path.dirname(filePath);
309-
const fullFilePath = path.join(projectDir, path.join(filePath, PACKAGE_JSON_FILE_NAME));
310-
if (this.$fs.exists(fullFilePath)) {
311-
const json = this.$fs.readJson(fullFilePath);
312-
if (json["nativescript"] && _.startsWith(file, path.join(filePath, "platforms"))) {
313-
return true;
314-
}
315-
}
316-
}
317-
}
318-
return false;
319-
}
320268
}
321269
$injector.register("projectChangesService", ProjectChangesService);

‎test/project-changes-service.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class ProjectChangesServiceTest extends BaseServiceTest {
3636
});
3737
this.injector.register("logger", LoggerStub);
3838
this.injector.register("hooksService", HooksServiceStub);
39+
this.injector.register("nodeModulesDependenciesBuilder", {});
3940

4041
const fs = this.injector.resolve<IFileSystem>("fs");
4142
fs.writeJson(path.join(this.projectDir, Constants.PACKAGE_JSON_FILE_NAME), {

‎test/stubs.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -506,16 +506,10 @@ export class ProjectDataService implements IProjectDataService {
506506
removeDependency(dependencyName: string): void { }
507507

508508
getProjectData(projectDir: string): IProjectData {
509-
return <any>{
510-
projectDir: "/path/to/my/projecDir",
511-
projectName: "myTestProjectName",
512-
platformsDir: "/path/to/my/projecDir/platforms",
513-
projectIdentifiers: {
514-
ios: "org.nativescript.myiosApp",
515-
android: "org.nativescript.myAndroidApp"
516-
},
517-
getAppResourcesRelativeDirectoryPath: () => "/path/to/my/projecDir/App_Resources"
518-
};
509+
const projectData = new ProjectDataStub();
510+
projectData.initializeProjectData(projectDir);
511+
512+
return projectData;
519513
}
520514

521515
async getAssetsStructure(opts: IProjectDir): Promise<IAssetsStructure> {

0 commit comments

Comments
 (0)