Skip to content
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

fix(platform-ios): detect Podfile and .xcworkspace #1436

Closed
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
19 changes: 0 additions & 19 deletions packages/platform-ios/src/config/__tests__/findPodfilePath.test.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*
*/

import findProject from '../findProject';
import {findXcodeProject as findProject} from '../findProject';
import * as projects from '../__fixtures__/projects';

jest.mock('path');
Expand Down
17 changes: 0 additions & 17 deletions packages/platform-ios/src/config/findPodfilePath.ts

This file was deleted.

44 changes: 32 additions & 12 deletions packages/platform-ios/src/config/findProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@

import glob from 'glob';
import path from 'path';
import isXcodeProject from './isXcodeProject';

/**
* Glob pattern to look for xcodeproj
*/
const GLOB_PATTERN = '**/*.xcodeproj';
const GLOB_PATTERN = '**/*.{xcodeproj,xcworkspace}';

/**
* Regexp matching all test projects
Expand All @@ -29,17 +30,9 @@ const IOS_BASE = 'ios';
*/
const GLOB_EXCLUDE_PATTERN = ['**/@(Pods|node_modules|Carthage)/**'];

/**
* Finds iOS project by looking for all .xcodeproj files
* in given folder.
*
* Returns first match if files are found or null
*
* Note: `./ios/*.xcodeproj` are returned regardless of the name
*/
export default function findProject(folder: string): string | null {
const projects = glob
.sync(GLOB_PATTERN, {
function findProject(folder: string, pattern: string): string[] {
Copy link
Member

Choose a reason for hiding this comment

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

I guess this function can now be renamed to glob or findInFolder, since it no longer finds a project, but acts as a utility function.

return glob
.sync(pattern, {
cwd: folder,
ignore: GLOB_EXCLUDE_PATTERN,
})
Expand All @@ -48,10 +41,37 @@ export default function findProject(folder: string): string | null {
path.dirname(project) === IOS_BASE || !TEST_PROJECTS.test(project),
Copy link
Member

Choose a reason for hiding this comment

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

This one is not related to your PR per se, but I stared wondering whether path.dirname(project) === IOS_BASE should be there or not.

Now, I don't remember why I ended up leaving this one here, instead of using glob pattern ios/**/*.{xcodeproj,xcworkspace}.

I actually wonder why do we enforce iOS folder in first place here. That was probably to ignore node_modules and additional projects that might be there.

)
.sort((project) => (path.dirname(project) === IOS_BASE ? -1 : 1));
}

/**
* Finds a `Podfile` in given folder.
*
* Returns first match if files are found or null
*
* Note: `./ios/Podfile` are returned regardless of the name
*/
export function findPodfile(folder: string): string | null {
const projects = findProject(folder, '**/Podfile');
Copy link
Member

Choose a reason for hiding this comment

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

Rename projects to podfiles or pods?

if (projects.length === 0) {
return null;
}

return projects[0];
}

/**
* Finds iOS project by looking for all .xcodeproj files
* in given folder.
*
* Returns first match if files are found or null
Copy link
Member

Choose a reason for hiding this comment

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

I think the description needs to be updated. How does it work now? Returns first element for which isXcodeProject returns true, or first element in an array? When exactly first element in an array is a valid return value?

*
* Note: `./ios/*.xcodeproj` are returned regardless of the name
*/
export function findXcodeProject(folder: string): string | null {
const projects = findProject(folder, GLOB_PATTERN);
if (projects.length === 0) {
return null;
}

return projects.find(isXcodeProject) || projects[0];
}
29 changes: 20 additions & 9 deletions packages/platform-ios/src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@

import path from 'path';
import {memoize} from 'lodash';
import findProject from './findProject';
import findPodfilePath from './findPodfilePath';
import {findPodfile, findXcodeProject} from './findProject';
import findPodspec from './findPodspec';
import isXcodeProject from './isXcodeProject';
import {
IOSProjectParams,
IOSDependencyParams,
} from '@react-native-community/cli-types';

const memoizedFindProject = memoize(findProject);
const memoizedFindPodfile = memoize(findPodfile);
const memoizedFindProject = memoize(findXcodeProject);

/**
* For libraries specified without an extension, add '.tbd' for those that
Expand All @@ -38,31 +39,41 @@ export function projectConfig(folder: string, userConfig: IOSProjectParams) {
if (!userConfig) {
return;
}

const podfile = memoizedFindPodfile(folder);
const project = userConfig.project || memoizedFindProject(folder);

/**
* No iOS config found here
*/
if (!project) {
if (!project && !podfile) {
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be !project || !podfile instead? I guess a project is not valid when .xcodeproj is missing.

On a side note, I guess with this change, having a Podfile will be mandatory.


const projectPath = path.join(folder, project);
const sourceDir = path.dirname(projectPath);
const projectPath = project && path.join(folder, project);

// Source files may be referenced from anywhere in a `.xcodeproj`. CocoaPods,
// on the other hand, is a lot stricter because files cannot live outside the
// folder in which `Podfile` resides. If we found a `Podfile`, it's a strong
// indication that source files are in that directory.
const sourceDir = path.resolve(path.dirname(podfile || projectPath || ''));

return {
sourceDir,
folder,
pbxprojPath: path.join(projectPath, 'project.pbxproj'),
podfile: findPodfilePath(projectPath),
pbxprojPath:
projectPath && isXcodeProject(projectPath)
? path.join(projectPath, 'project.pbxproj')
: null,
podfile: podfile && path.resolve(podfile),
Copy link
Member

Choose a reason for hiding this comment

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

Do we need podfile && here, if we enforce it on line 49?

Copy link
Member

Choose a reason for hiding this comment

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

Hm right, with !project && !podfile guard we don't need to check either of those

podspecPath:
userConfig.podspecPath ||
// podspecs are usually placed in the root dir of the library or in the
// iOS project path
findPodspec(folder) ||
findPodspec(sourceDir),
projectPath,
projectName: path.basename(projectPath),
projectName: projectPath && path.basename(projectPath),
libraryFolder: userConfig.libraryFolder || 'Libraries',
sharedLibraries: mapSharedLibaries(userConfig.sharedLibraries || []),
plist: userConfig.plist || [],
Expand Down
10 changes: 10 additions & 0 deletions packages/platform-ios/src/config/isXcodeProject.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

export default function isXcodeProject(projectPath: string): boolean {
return projectPath.endsWith('.xcodeproj');
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ export default function warnAboutManuallyLinkedLibs(
Config['platforms']['ios']['linkConfig']
> = getLinkConfig(),
) {
let deps: Array<string> = [];
const projectConfig = config.project[platform];
if (projectConfig.pbxprojPath === null) {
return;
}

let deps: Array<string> = [];

for (let key in config.dependencies) {
const dependency = config.dependencies[key];
Expand Down