feat: add report option to cli

[BioPhoton]

This PR includes:

• add persist.report to model
• add persist.report to CLI options and logic
• terminal output is perserved to be cached by Nx

Related:
#1048

[github-actions]

Code PushUp

🤨 Code PushUp report has both improvements and regressions – compared current commit ab0d296 with previous commit ae52e3b.

🕵️ See full comparison in Code PushUp portal 🔍

🏷️ Categories

🏷️ Category	⭐ Previous score	⭐ Current score	🔄 Score change
Performance	🔴 31	🔴 32
Code coverage	🟢 90	🟢 90
Security	🔴 0	🔴 0	–
Updates	🟡 76	🟡 76	–
Accessibility	🟢 92	🟢 92	–
Best Practices	🟢 100	🟢 100	–
SEO	🟡 61	🟡 61	–
Type Safety	🟢 100	🟢 100	–
Bug prevention	🟢 100	🟢 100	–
Miscellaneous	🟢 100	🟢 100	–
Code style	🟢 100	🟢 100	–
Documentation	🔴 23	🔴 23	–

👍 2 groups improved, 👍 5 audits improved, 👎 1 audit regressed, 14 audits changed without impacting score

🗃️ Groups

🔌 Plugin	🗃️ Group	⭐ Previous score	⭐ Current score	🔄 Score change
Lighthouse	Performance	🔴 31	🔴 32
Code coverage	Code coverage metrics	🟢 90	🟢 90

19 other groups are unchanged.

🛡️ Audits

🔌 Plugin	🛡️ Audit	📏 Previous value	📏 Current value	🔄 Value change
Lighthouse	First Contentful Paint	🟥 3.2 s	🟥 3.0 s
Lighthouse	Speed Index	🟥 8.4 s	🟥 8.3 s
Lighthouse	Time to Interactive	🟥 18.6 s	🟥 19.3 s
Code coverage	Branch coverage	🟨 85 %	🟨 85.1 %
Code coverage	Line coverage	🟨 86.7 %	🟨 86.8 %
Code coverage	Function coverage	🟩 93.1 %	🟩 93.1 %
Lighthouse	Avoids enormous network payloads	🟩 Total size was 2,649 KiB	🟩 Total size was 2,644 KiB
Lighthouse	Minimizes main-thread work	🟥 21.9 s	🟥 21.0 s
Lighthouse	Server Backend Latencies	🟩 1,050 ms	🟩 220 ms
Lighthouse	Metrics	🟩 100%	🟩 100%
Lighthouse	Uses efficient cache policy on static assets	🟨 30 resources found	🟨 30 resources found
Lighthouse	Largest Contentful Paint	🟥 12.5 s	🟥 12.1 s
Lighthouse	Max Potential First Input Delay	🟥 2,400 ms	🟥 2,080 ms
Lighthouse	Reduce unused CSS	🟥 Potential savings of 92 KiB	🟥 Potential savings of 102 KiB
Lighthouse	Total Blocking Time	🟥 7,080 ms	🟥 7,000 ms
Lighthouse	Reduce unused JavaScript	🟥 Potential savings of 579 KiB	🟥 Potential savings of 600 KiB
Lighthouse	Network Round Trip Times	🟩 10 ms	🟩 40 ms
Lighthouse	Remove duplicate modules in JavaScript bundles	🟥 Potential savings of 88 KiB	🟥 Potential savings of 88 KiB
Lighthouse	JavaScript execution time	🟥 10.4 s	🟥 10.4 s
Lighthouse	Initial server response time was short	🟩 Root document took 510 ms	🟩 Root document took 490 ms

590 other audits are unchanged.

[matejchalk]

The implementation is good, but the naming doesn't feel right to me. Suggested a few alternatives.

[matejchalk]

The way this option is named and described feels off to me. Although we document --persist.report, we don't expect anyone to ever use it like that, the only sensible usage is --persist.no-report, which itself looks weird and I don't expect it'll occur to users to use the no- "prefix" like that (why not --no-persist.report for example?). Another oddity is that although it sits under persist namespace on the same level with other options, disabling it actually makes all the other persist options meaningless (not clear from the name). And I don't think persist.report works linguistically either, because we're chaining two verbs mean the same thing ("generate a file").

However, I can see why your chose to combine it with existing persist options (logically coupled) and included no- prefix negation (consistency for boolean options), instead of a standalone --no-report option.

I can think of 2 alternatives which would be more intuitive:

1. Instead of adding a new property under persist, we could allow setting persist: false (config) and --no-persist (CLI). The type definition would effectively be type PersistConfig = false | { outputDir?: string, ... } (default is the object, i.e. enabled with default options). This nicely illustrates how the logic works - either you don't persist anything, or you do and only in that case does it make sense to able to configure options. Our implementation can then be a simple truthiness check. And a --no-persist flag is fairly self-explanatory.
2. If we want to keep persist as an object, then I'd rename the property and invert its meaning. I'd go with persist.disabled. This way the default value is false (always preferable with booleans, aligns with truthy/falsy JS logic), it's more obvious from the name what it does and the documentation is less awkward. Now, we seem to be claiming --persist.report is "useful in combination with caching" - it isn't, --persist.no-report is, but that's not at all clear - but this description would work just fine for --persist.disabled.