Update no jitpack buld for monorepo #23325
Conversation
Also added a check to insure that this kind of mistake gets caught quickly in the future since otherwise it only reveals itself at runtime.
mchowning
added
the
Mobile App - i.e. Android or iOS
|
Size Change: 0 B Total Size: 1.12 MB ℹ️ View Unchanged
|
| } | ||
| } | ||
|
|
||
| def isRelevantFile = { it.name.endsWithAny('.js', '.css') || it.name ==~ /package(-lock)?\.json/ } |
There was a problem hiding this comment.
This check has been working for us so far, but I wouldn't be surprised at all if there's a filetype that we've missed. Let me know if you can think of any other "relevant" files we should be checking for changes.
There was a problem hiding this comment.
Maybe we should include ".scss", or just make it ends with "css" (without the ".")?
There was a problem hiding this comment.
👍 Good catch. I made the update in afc4d52. Went with adding ".scss" just for the sake of being super-explicit.
| inputs.files(inputFiles()) | ||
| def dirs = [jsRootDir] | ||
| file(jsRootDir).eachDirRecurse { dir -> | ||
| def isRelevantDir = 
|
This is definitely looking better. One thing I'm seeing, is that the second build after making a change doesn't seem to skip the npm_ci / buildJSBundle task as expected. See steps below:
|
I'm seeing a couple of things that could be causing this. 1. Package.json formatting changes get undoneOne that I just realized is that 2. Package-lock.json is getting modified by the buildA second one is that I am seeing a change being introduced in is being changed to: during the build. So the first build sees the original line, then the second build sees that it changed and redoes everything. The third build finally sees no changes for the first time and gives us a fast(er) build. I'm not seeing the long-second-build issue with these steps:
If undoing the change to |
Or we could probably just stop checking the package-lock.json files for changes since I believe checking the package.json files should be sufficient. |
|
Pushing 2 updates:
|
mchowning
force-pushed
the
fix_no_jitpack_build
branch
from
5a60bfe to
0d0bc36
Compare
mchowning
force-pushed
the
fix_no_jitpack_build
branch
from
0d0bc36 to
47fc9a6
Compare
|
I can confirm that multiple invocations of Also, I can confirm that the non-from-source-mode WPAndroid app runs OK and the editor launches OK too without Metro running. While at it @mchowning , what do you think about revising the "Building the Gutenberg Mobile JS bundle" message in this PR? With all the build optimizations in place, that message is a bit misleading. We could prolly change it to "Executing nested Gutenberg Mobile build tasks..." or something. |
Certainly up for improving that message, but I feel like if I saw "Executing nested Gutenberg Mobile build tasks..." there's a decent chance I would be misled into thinking that it was a from-source build ( You mentioned that the current message is a bit misleading "with all the build optimizations in place" @hypest . Would you mind explaining a bit how it is misleading? I think that might help me think of some better wording for the message. |
Oh, so, the current one ("Building the Gutenberg Mobile JS bundle") to me reads like the JS is actually being built, and the JS/RN tools being run. I would expect to see that printed only if the JS bundle is (re)built.
Fair. Note, this is not a blocker and we can work on it in a follow up PR after the monorepo things get merged. |
|
Regarding the "Building the Gutenberg Mobile JS bundle" message, I don't feel strongly but unless you have a suggestion you feel good or strongly about @hypest , let's just leave this to be addressed later. I haven't been able to come up with a message I like yet, and there's a decent chance this will be changing a bit anyway in the near future due to jitpack, so that will provide a good opportunity to improve the messaging. |
mchowning
force-pushed
the
fix_no_jitpack_build
branch
from
90aa339 to
857f407
Compare
There was a problem hiding this comment.
Nice fixes @mchowning !
✅ 3. Run the app and confirm you can open the Gutenberg editor without crashing (establishes that the bundle is being included in the apk)
First ^ build took 7m 9s for me.
✅ 5. Observe that this build takes under 1 minute whereas the first build took multiple minutes because this build is not regenerating the bundle file (establishes that the build detected there were no changes that would affect the bundle file)
Second build took 28s for me 🎉
✅ 6. Edit any file within the gutenberg-mobile or gutenberg projects that should alter the resulting bundle other than a file within a node_modules/ folder (the build is just checking the package.json and package-lock.json files for dependency changes).
Added a comment in the root src/index.js file.
✅ Observe that the app takes multiple minute to build this time because it detected the change and is regenerating the bundle (establishes that the build detected there were no changes that would affect the bundle file).
^ took 2m 7s for me.
✅ 10. Run the app and confirm you can open the Gutenberg editor without crashing (establishes does not just check whether the bundle file exists at the beginning of the build, but instead checks it after the "clean" task has run).
^ took 2m 54s for me and app/editor ran OK on device.
✅ 12. Run the app and confirm you can open the Gutenberg editor without crashing.
LGTM! 
|
Will merge with "preserve commits" now. |
Description
This PR addresses two issues that were found in the no-jitpack WPAndroid builds with the monorepo:
In addition, while investigating the always-rebuilding issue, I discovered a problem where the bundle would not get properly generated when a build was run with the clean task. That is also fixed in this PR.
How has this been tested?
For all of these tests:
gradle.propertiesfile. These changes should have no effect when that variable is set totrue; andThis is just a proposed set of commands that should cover all of the edge cases I have encountered, but it's not a bad idea to try different combinations since that could catch something I missed. All these commands should be run from the monorepo branch for WPAndroid, but with this
fix_no_jitpack_buildbranch checked out for gutenberg../gradlew clean./gradlew installWasabiDebug./gradlew installWasabiDebugnode_modules/folder (the build is just checking the package.json and package-lock.json files for dependency changes)../gradlew installWasabiDebug./gradlew clean installWasabiDebugVanillavariant. Ideally, do a release build, but if your environment isn't set up for that a debug build is fine:./gradlew installVanillaReleaseChecklist: