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

[tools] New config-based vendoring that supports Swift #11488

Merged
merged 6 commits into from Jan 9, 2021

Conversation

tsapeta
Copy link
Member

@tsapeta tsapeta commented Dec 30, 2020

Why

Current vendoring tool is pretty old, no longer meets our requirements and it overcomplicates many things.
But right now, the main reason to rewrite it is the need to upgrade lottie-react-native that we've been blocked for a long time because it's written in Swift as of v3. Our tools don't know how to vendor and how to version Swift files.

How

I've noticed that vendoring and versioning have a lot in common — their main duty is to copy things from directory A to directory B with some file renames and file content transformations.
So why not to implement them in the same way? In iOS versioning we already have a post-transform step that is entirely based on the config file that contains easy-to-read transform rules of such type:

{
  /**
   * A substring or RegExp matching a part of the input that you want to replace.
   */
  find: RegExp | string;

  /**
   * A string that replaces matched substrings in the input.
   */
  replaceWith: string | ((substring: string, ...args: any[]) => string);

  /**
   * An array of glob patterns matching files to which the transform should be applied.
   */
  paths: string | string[];
}

This was quite good so far, so I decided to do the new vendoring in the same way.

Also, vendored modules were so far being copied to app's source files — which I think was maybe good for ExpoKit, but it doesn't seemed right to me. To support Swift, it would be much easier to have them installed as separate pods (and Android libraries for consistency) not only because it solves the biggest problem with versioning Swift modules. When installed as separate libraries we don't have to rename Swift files nor prefix any code references that would generate duplicated symbols. In Swift, the module name is a part of the symbol, so instead of prefixing every class/method/consts with EX (in unversioned code) we could just prefix the module name. Yupi! 🎉

Appendix: the new command shows a list of outdated modules in a table 💪

Test Plan

The new config contains only lottie-react-native, so I run
et new-update-vendored-module -m lottie-react-native and confirmed the library was cloned to temporary folder and then copied to ios/vendored/unversioned and android/vendored/unversioned folders, as expected. In this PR they're still not being installed.

tools/src/vendoring/config/index.ts Outdated Show resolved Hide resolved
tools/src/StringTransforms.ts Outdated Show resolved Hide resolved
tools/src/StringTransforms.ts Outdated Show resolved Hide resolved
@tsapeta tsapeta marked this pull request as ready for review January 5, 2021 09:37
@tsapeta tsapeta requested a review from bbarthec January 5, 2021 09:37
Copy link
Contributor

@bbarthec bbarthec left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍
Left a few small comments 💅

tools/src/Utils.ts Outdated Show resolved Hide resolved
tools/src/Utils.ts Show resolved Hide resolved
tools/src/Transforms.ts Outdated Show resolved Hide resolved
tools/src/Transforms.ts Outdated Show resolved Hide resolved
tools/src/Transforms.ts Outdated Show resolved Hide resolved
)
.option('--no-pbxproj', 'Whether to skip updating project.pbxproj file.', false)
.asyncAction(action);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not checking this file, as it's name suggests it's leftover from the ongoing process of refactoring this script, am I right? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

This is in the old, legacy version of the command where the vendored modules were added directly to the project and not as a pod and because of that any new files have to be added to pbxproj file. The new vendoring command won't need to do this tho.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants