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

refactor(breaking): remove deprecated link, unlink and associated code #1537

Merged
merged 40 commits into from
Feb 7, 2022
Merged
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
e151563
chore: initial commit - remove files
grabbou Jan 27, 2022
0ae9eea
chore: further removal
grabbou Jan 27, 2022
30103d3
chore: wip
grabbou Jan 27, 2022
6c53e48
continue work
grabbou Jan 28, 2022
596d849
chore: fix warnings and errors
grabbou Jan 28, 2022
8a24cd2
chore: move findXcodeProj to config and upgrade run-ios
grabbou Feb 2, 2022
a1fe895
chore: add todo
grabbou Feb 2, 2022
2f8baf8
chore: remove example
grabbou Feb 2, 2022
510a903
chore: update source dir and update dependnecy config for ios
grabbou Feb 2, 2022
2a43345
chore: remove logger
grabbou Feb 2, 2022
410ec1a
chore: fix type issues
grabbou Feb 2, 2022
b3e0730
chore: remove tests for missing properties, prefer snapshots instead …
grabbou Feb 2, 2022
16996b3
chore: update snapshots for iOS config (removed properties)
grabbou Feb 2, 2022
a67a5a6
chore: fix upgrade tests
grabbou Feb 2, 2022
65f4335
chore: remove extra tests
grabbou Feb 2, 2022
edf3175
chore: update config tests
grabbou Feb 2, 2022
3854ee3
chore: two tests tbd to support new resolution mechanism
grabbou Feb 2, 2022
f8a9ade
chore: fix ios tests and bring back configurable sourceDir
grabbou Feb 2, 2022
1c9508e
feat: align findPodfilePath with old findProject heuristics
grabbou Feb 2, 2022
be963e1
chore: fix lint
grabbou Feb 2, 2022
194ee75
chore: update snapshot
grabbou Feb 2, 2022
387f9ee
fix: filter invalid deps
grabbou Feb 2, 2022
299285f
chore: update tests
grabbou Feb 2, 2022
d665720
chore: add missing properties to Joi schema
grabbou Feb 2, 2022
020aa16
chore: another update
grabbou Feb 2, 2022
0153690
chore: fix Joi schema
grabbou Feb 2, 2022
f8fd19d
chore: update schema
grabbou Feb 2, 2022
de1a5d2
chore: fix snapshot
grabbou Feb 2, 2022
2c732a8
chore: note on the future development for this file
grabbou Feb 2, 2022
6051201
chore: update snapshot one more time - nitpick
grabbou Feb 2, 2022
361f0c9
one last time
grabbou Feb 2, 2022
64b4ef7
feat: print when multiple podfiles are found
grabbou Feb 3, 2022
00ccd36
update docs
grabbou Feb 3, 2022
59cb6e0
chore: fix
grabbou Feb 3, 2022
9e70eb3
Update autolinking.md
grabbou Feb 4, 2022
48686a9
chore: remove xmldoc dep
thymikee Feb 4, 2022
7326892
chore: remove xcode dep
thymikee Feb 4, 2022
794d9b3
chore: return type for dependencyConfig
thymikee Feb 4, 2022
ae7c66e
Merge branch 'master' into feat/remove-link
grabbou Feb 7, 2022
3e5cb0b
Update index.ts
grabbou Feb 7, 2022
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
Prev Previous commit
Next Next commit
feat: align findPodfilePath with old findProject heuristics
  • Loading branch information
grabbou committed Feb 2, 2022
commit 1c9508e62a34ad316919612928e26642379ce171
42 changes: 36 additions & 6 deletions packages/platform-ios/src/config/findPodfilePath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,43 @@
import glob from 'glob';
import path from 'path';

export default function findPodfilePath(folder: string) {
const podfiles = glob.sync('**/Podfile', {
cwd: folder,
ignore: 'node_modules/**',
});
// Regexp matching all test projects
const TEST_PROJECTS = /test|example|sample/i;

// Base iOS folder
const IOS_BASE = 'ios';

// These folders will be excluded from search to speed it up
const GLOB_EXCLUDE_PATTERN = ['**/@(Pods|node_modules|Carthage)/**'];

export default function findPodfilePath(cwd: string) {
/**
* First, we're going to look for all Podfiles within the `cwd`
*/
const podfiles = glob
.sync('**/Podfile', {
cwd,
ignore: GLOB_EXCLUDE_PATTERN,
})
/**
* Then, we will run a simple test to rule out most example projects,
* unless they are located in a `ios` folder
*/
.filter(
(project) =>
path.dirname(project) === IOS_BASE || !TEST_PROJECTS.test(project),
)
/**
* Podfile from `ios` folder will be picked up as a first one.
*/
.sort((project) => (path.dirname(project) === IOS_BASE ? -1 : 1));

if (podfiles.length > 0) {
return path.join(folder, podfiles[0]);
/**
* @todo
* Shall we print a warning when multiple `Podfile` were found?
Copy link
Contributor

@tido64 tido64 Feb 3, 2022

Choose a reason for hiding this comment

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

I ran this locally and got:

> require("glob").sync('**/Podfile', { ignore: ['**/@(Pods|node_modules|Carthage)/**'] })
[ 'ios/Podfile', 'macos/Podfile' ]

I think printing a warning can be very helpful, but there can be false positives if you also have a macOS project. Maybe we could print the list if --verbose is specified?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like this idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

We will favour iOS anyway since this is iOSConfig used for run-ios. But I guess we can adapt it in a later PR to work for Mac too?

Can you point me to a configuration/helpers that you use for macos?

Copy link
Contributor

Choose a reason for hiding this comment

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

findXcodeProject is here: https://github.com/microsoft/react-native-macos/blob/main/local-cli/runMacOS/findXcodeProject.js

It only gets called from run-macos (code), so it can assume that pod install has happened. You wouldn't be able to use it for config.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, do you want me to export you this helper so you can reduce some code overlap? Or prefer to keep it internal?

Choose a reason for hiding this comment

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

@HeyImChris I believe we search for podfiles rather than Xcode projects to support scenarios like react-native-test-app, which generates an xcode project given a podfile. In effect, the source of truth becomes the podfile and not the xcode project. I remember when @tido64 pushed for this change in the CLI which in turn helped us use react-native-test-app for FluentUI React Native.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Saadnajmi there's a sub-package @react-native-community/platform-ios. React Native depends on it. CLI doesn't. CLI detects runtime packages with platforms (such as platform-ios, or react-native-windows) and adds commands / configurations based on installed packages.

So by depending on platform-ios, you wouldn't pull the entire CLI.

Choose a reason for hiding this comment

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

I think this change makes sense given that we wouldn't be taking on a bunch of new dependencies with the obvious value of reducing our diffs. @Saadnajmi sound good to you?

Choose a reason for hiding this comment

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

As it turns out, we already have a dependency on the CLI (thanks @HeyImChris for pointing it out) so yeah this would be great! Yay code sharing!

Copy link
Member Author

Choose a reason for hiding this comment

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

Right -> https://github.com/microsoft/react-native-macos/blob/26a7c3e9d430ebb58b9c9e2aa220fe9019ae1e1f/package.json#L99.

I will open a follow-up issue to explore various code-sharing opportunities.

*/
return path.join(cwd, podfiles[0]);
}

return null;
Expand Down