Updated unused style keys script - #65943
Conversation
|
|
roryabraham
left a comment
There was a problem hiding this comment.
Can you write unit tests please? You can look at generateTranslationTest and TSCompilerUtilsTest for inspiration
roryabraham
left a comment
There was a problem hiding this comment.
plus ESLint is failing. Overall, this is looking promising! Thanks for all your work on this
|
Awesome, thanks for the feedback! I remember that I tested some of those suggestions in the past and was getting some false positives. I will check them again now that everything is working, as they might work properly now. |
|
@roryabraham Thanks for the feedback! I applied your suggestions and learned a lot about cases I hadn't used enough before. |
roryabraham
left a comment
There was a problem hiding this comment.
Overall looking much better to me!
Co-authored-by: Rory Abraham <47436092+roryabraham@users.noreply.github.com>
Co-authored-by: Rory Abraham <47436092+roryabraham@users.noreply.github.com>
|
@roryabraham feedback done, can we run again the action? not sure why found a style, I'm checking locally and is working |
|
NAB but I might not mind if we put this in a separate workflow from ESLint. ESLint is taking a long time these days, so the feedback loop for this script in CI is pretty slow too |
|
@gedu check is still failing. Try merging main? |
|
Ohh the style |
|
Yeah, let's add a new workflow. Separate (parallel) checks is better than long-running workflows |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.1.83-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.1.83-5 🚀
|


Explanation of Change
I migrated the solution because the issue was using regex for complex syntax parsing instead of simple text pattern matching where it excels. I switched to TypeScript AST for structure analysis combined with targeted regex for text searches, rather than having regex try to do everything.
Fixed Issues
$ #63192 (comment)
PROPOSAL: -
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop