Skip to content

make livesync work with renamed files and folders #3042

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 8 commits into from
Aug 17, 2017
Merged

make livesync work with renamed files and folders #3042

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
94eeebb
added special check for file changes on Mac
Plamen5kov Aug 9, 2017
d7f06d2
fix missing imports in tests
Plamen5kov Aug 9, 2017
3692d8b
add dir check, modified file check
Plamen5kov Aug 10, 2017
c861d06
check isDirModified before anything else to save time
Plamen5kov Aug 10, 2017
3b8cb02
updates after review
Plamen5kov Aug 11, 2017
3c2713c
bring back check for symlinks
Plamen5kov Aug 14, 2017
9020a86
add check for modification in bot files and folders
Plamen5kov Aug 14, 2017
f99697d
fixed lint
Plamen5kov Aug 14, 2017
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
19 changes: 15 additions & 4 deletions lib/services/project-changes-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export class ProjectChangesService implements IProjectChangesService {
path.join(projectData.projectDir, NODE_MODULES_FOLDER_NAME, "tns-ios-inspector"),
projectData,
this.fileChangeRequiresBuild);

if (this._newFiles > 0) {
this._changesInfo.modulesChanged = true;
}
Expand Down Expand Up @@ -218,19 +219,23 @@ export class ProjectChangesService implements IProjectChangesService {
}

private containsNewerFiles(dir: string, skipDir: string, projectData: IProjectData, processFunc?: (filePath: string, projectData: IProjectData) => boolean): boolean {
const dirFileStat = this.$fs.getFsStats(dir);
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 lstat of the dir has been changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well spotted, I don't know how I missed that!

if (this.isFileModified(dirFileStat)) {
return true;
}

let files = this.$fs.readDirectory(dir);
for (let file of files) {
let filePath = path.join(dir, file);
if (filePath === skipDir) {
continue;
}

let fileStats = this.$fs.getFsStats(filePath);

let changed = fileStats.mtime.getTime() >= this._outputProjectMtime || fileStats.ctime.getTime() >= this._outputProjectCTime;
const fileStats = this.$fs.getFsStats(filePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

the fileStats object is not used anywhere outside of the isFileModified method. So I believe its place is inside it. At the moment on line 222 we have:

  1. get fstat
  2. call isFileModified

Here we are doing the same:

  1. get fstat
  2. call isFileModified

I suggest passing only the path to isFileModified and getting the fstat in the method itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used here

let changed = this.isFileModified(fileStats);
if (!changed) {
let lFileStats = this.$fs.getLsStats(filePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this code deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's unnecessary to do this.$fs.getLsStats(filePath) twice and the logic is kept the same in the refactored version

Copy link
Contributor

Choose a reason for hiding this comment

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

getLstStats and getFsStats are different. This code is used when the file/dir is symlink - we need to check the symlink itself (for example if it is recreated) and this is done by getLsStats.

Copy link
Contributor

Choose a reason for hiding this comment

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

ping @Plamen5kov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will bring back the code, so we continue working with synlinked files.

changed = lFileStats.mtime.getTime() >= this._outputProjectMtime || lFileStats.ctime.getTime() >= this._outputProjectCTime;
changed = this.isFileModified(lFileStats);
}

if (changed) {
Expand All @@ -250,10 +255,16 @@ export class ProjectChangesService implements IProjectChangesService {
return true;
}
}

}
return false;
}

private isFileModified(filePathStat: IFsStats): boolean {
return filePathStat.mtime.getTime() >= this._outputProjectMtime ||
filePathStat.ctime.getTime() >= this._outputProjectCTime;
}

private fileChangeRequiresBuild(file: string, projectData: IProjectData) {
if (path.basename(file) === "package.json") {
return true;
Expand Down
1 change: 0 additions & 1 deletion test/project-changes-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class ProjectChangesServiceTest extends BaseServiceTest {
this.injector.register("platformsData", PlatformsData);
this.injector.register("androidProjectService", {});
this.injector.register("iOSProjectService", {});

this.injector.register("fs", FileSystem);
this.injector.register("devicePlatformsConstants", {});
this.injector.register("devicePlatformsConstants", {});
Expand Down