Skip to content

fix(prepare): disregard app_resources's dir name when preparing in platforms/ #3403

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.

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
Mar 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 6 additions & 6 deletions lib/services/app-files-updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ export class AppFilesUpdater {

public updateApp(updateAppOptions: IUpdateAppOptions, projectData: IProjectData): void {
this.cleanDestinationApp(updateAppOptions);
const sourceFiles = updateAppOptions.filesToSync || this.resolveAppSourceFiles(projectData);
let sourceFiles = updateAppOptions.filesToSync || this.resolveAppSourceFiles(projectData);

// exclude the app_resources directory from being enumerated
// for copying if it is present in the application sources dir
const appResourcesPathNormalized = path.normalize(projectData.appResourcesDirectoryPath + "\\");
sourceFiles = sourceFiles.filter(dirName => !path.normalize(dirName).startsWith(appResourcesPathNormalized));

updateAppOptions.beforeCopyAction(sourceFiles);
this.copyAppSourceFiles(sourceFiles);
Expand Down Expand Up @@ -76,11 +81,6 @@ export class AppFilesUpdater {
constants.LIVESYNC_EXCLUDED_FILE_PATTERNS.forEach(pattern => sourceFiles = sourceFiles.filter(file => !minimatch(file, pattern, { nocase: true })));
}

// exclude the app_resources directory from being enumerated
// for copying if it is present in the application sources dir
const appResourcesPathNormalized = path.normalize(projectData.appResourcesDirectoryPath);
sourceFiles = sourceFiles.filter(dirName => !path.normalize(dirName).startsWith(appResourcesPathNormalized));

return sourceFiles;
}

Expand Down
4 changes: 2 additions & 2 deletions lib/services/prepare-platform-js-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class PreparePlatformJSService extends PreparePlatformService implements
if (config.changesInfo && !config.changesInfo.changesRequirePrepare) {
// remove the App_Resources folder from the app/assets as here we're applying other files changes.
const appDestinationDirectoryPath = path.join(config.platformData.appDestinationDirectoryPath, constants.APP_FOLDER_NAME);
const appResourcesDirectoryPath = path.join(appDestinationDirectoryPath, constants.APP_RESOURCES_FOLDER_NAME);
const appResourcesDirectoryPath = path.join(appDestinationDirectoryPath, path.basename(config.projectData.appResourcesDirectoryPath));
if (this.$fs.exists(appResourcesDirectoryPath)) {
this.$fs.deleteDirectory(appResourcesDirectoryPath);
}
Expand Down Expand Up @@ -107,7 +107,7 @@ export class PreparePlatformJSService extends PreparePlatformService implements
const appDestinationDirectoryPath = path.join(config.platformData.appDestinationDirectoryPath, constants.APP_FOLDER_NAME);
const appResourcesSourcePath = config.projectData.appResourcesDirectoryPath;

shell.cp("-Rf", appResourcesSourcePath, appDestinationDirectoryPath);
shell.cp("-Rf", appResourcesSourcePath, path.join(appDestinationDirectoryPath, constants.APP_RESOURCES_FOLDER_NAME));
}
}

Expand Down
1 change: 1 addition & 0 deletions lib/services/prepare-platform-native-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export class PreparePlatformNativeService extends PreparePlatformService impleme
private copyAppResources(platformData: IPlatformData, projectData: IProjectData): void {
const appDestinationDirectoryPath = path.join(platformData.appDestinationDirectoryPath, constants.APP_FOLDER_NAME);
const appResourcesDirectoryPath = path.join(appDestinationDirectoryPath, constants.APP_RESOURCES_FOLDER_NAME);

if (this.$fs.exists(appResourcesDirectoryPath)) {
platformData.platformProjectService.prepareAppResources(appResourcesDirectoryPath, projectData);
const appResourcesDestination = platformData.platformProjectService.getAppResourcesDestinationDirectoryPath(projectData);
Expand Down
4 changes: 2 additions & 2 deletions test/app-files-updates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require("should");
function createTestInjector(): IInjector {
const testInjector = new yok.Yok();

testInjector.register("projectData", { appResourcesDirectoryPath: "App_Resources"});
testInjector.register("projectData", { appResourcesDirectoryPath: "App_Resources" });

return testInjector;
}
Expand Down Expand Up @@ -71,7 +71,7 @@ describe("App files copy", () => {

it("copies all app files but app_resources when not bundling", () => {
const updater = new CopyAppFilesUpdater([
"file1", "dir1/file2", "App_Resources/Android/blah.png"
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 changed? If the behavior is changed, the testname and expected results should be 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.

@rosen-vladimirov that's an oversight on my part. I changed the only failing test, because the app_resources were no longer filtered as part of the copy() method but in public updateApp() which returns void and I found no straightforward way of mocking it in order to update the test properly.

"file1", "dir1/file2"
], { bundle: false });
updater.copy();
assert.deepEqual(["file1", "dir1/file2"], updater.copiedDestinationItems);
Expand Down