[module-scripts] vendoring source-login-scripts.sh #15336
Conversation
expo-bot
added
bot: passed checks
packages/expo-updates/package.json
Outdated
| @@ -12,6 +12,8 @@ | |||
| "test": "expo-module test", | |||
| "prepare": "expo-module prepare", | |||
| "prepublishOnly": "expo-module prepublishOnly", | |||
| "prepack": "expo-module vendorScripts", | |||
There was a problem hiding this comment.
The expo-module-scripts binaries are named after npm lifecycle scripts: expo-module prepack. The idea is that each package's package.json file shouldn't make many decisions -- they should just map each npm lifecycle script to expo-module <X>.
| # Keeps the following `$$EXPO_MODULE_VENDOR_SCRIPTS$$`` comment and | ||
| # `expo-module-scripts' will replace the comment line with the content of `source-login-scripts.sh` during npm prepack | ||
| source "$EXPO_UPDATES_PACKAGE_DIR/../../tools/source-login-scripts.sh" # $$EXPO_MODULE_VENDOR_SCRIPTS$$ '../../../tools/source-login-scripts.sh' |
There was a problem hiding this comment.
Dynamically vendoring the script seems fragile. I think it would be better just to copy/paste it and commit the vendored scripts to the repo (Git will deduplicate the files in its index).
There was a problem hiding this comment.
i agree that and what's the "vendoring" in your thought? the current work is exactly copy/paste the source-login-scripts.sh into expo-constants package:
There was a problem hiding this comment.
I have two ideas:
- Vendoring + committing the script to Git: Copy and paste source-login-scripts.sh into the packages that need it. If we need to, we could make
expo-module preparecheck ifscripts/source-login-scripts.shis defined and contains the string# @autogenerated. If so, it would copy the latest version of source-login-scripts.sh into the package, similar to how tsconfig.json, babel.config.js, etc. are copied. - Depending on the the script as an npm dependency: add source-login-scripts@^1.0.0 to package.json and source
$script_dir/../../node_modules/source-login-scripts/source-login-scripts.sh. However, this technique breaks with Yarn workspaces since the source-login-scripts package might be hoisted into the root directory.
There was a problem hiding this comment.
i like the option 1 because it won't break workspace support and easier to maintain. i'll update the pr accordingly. thanks for your advice 😁
expo-bot
added
bot: suggestions
Kudo
force-pushed
the
@kudo/vendoring-source-login-script
branch
from
d736bae to
a496c5c
Compare
|
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
good idea 👏 i've updated the pr for the symlink change. |
…po modules (#15376) # Why regression of #15336, where we should not sync `source-login-scripts.sh` to other expo modules. i just misread the `if [ ! -f "$output_file" ] || grep --quiet "@generated" "$output_file"; then` line. # How - introduce `source-login-scripts.sh` as an optional template file which only to sync if it's present and contains `@generated` as described in #15336 (comment) # Test Plan - `yarn prepare` in expo-device and make sure `source-login-scripts.sh` not be generated. - change `source-login-scripts.sh` and `yarn prepare` in expo-constants. check it's updated. Co-authored-by: James Ide <ide@users.noreply.github.com>
Why
follow up with #14895 (comment)
close ENG-2493
How
add
templates/scripts/source-login-scripts.shto expo-module-scripts and it will sync to expo module package whenyarn prepareif there's alsoscripts/source-login-scripts.shand having@generatedcomment mark.change the file iterator in
expo-module-configurefrom shell glob expansion withfindwhich enabled the recursive iteration, e.g.scripts/source-login-scripts.shTest Plan
yarn prepareforexpo-constants, make sure the files are synced.yare prepackin expo-updates and from a sdk43 project to runyarn add file:/path/to/expo/packages/expo-updates, test the build successful.Checklist
expo build(eg: updated@expo/xdl).expo prebuild& EAS Build (eg: updated a module plugin).