deps: upgrade trace_engine, reuse cdt bf-cache/deprecation strings #16362
Conversation
|
Important The terms of service for this installation has not been accepted. Please ask the Organization owners to visit the Gemini Code Assist Admin Console to sign it. |
core/scripts/i18n/collect-strings.js
Outdated
| strings[`node_modules/@paulirish/trace_engine/${key.replace('.ts', '.js')}`] = value; | ||
| const lhKey = `node_modules/@paulirish/trace_engine/${key.replace('.ts', '.js')}`; | ||
|
|
||
| // TODO: why is this so malformed? |
There was a problem hiding this comment.
Thought I recognized this haha. This looks like a translation bug that we fixed on the LH side but never upstreamed to DevTools: https://b.corp.google.com/issues/290679361
I guess it's a good thing we are using the TE strings now, so we can avoid fixing in one place but not the other. I will file a separate bug to resolve this again.
There was a problem hiding this comment.
Err, maybe this is a slightly different issue. The original DT string didn't contain any \ at all which is different from LH...
There was a problem hiding this comment.
I filed b/399706272 about this, this workaround is fine for now
| if (insight.state === 'fail') { | ||
| score = 0; | ||
| } else if (insightName === 'LCPPhases') { | ||
| // TODO: change these insights to denote passing/failing/informative. Until then... hack it. |
There was a problem hiding this comment.
We have informative state now. What's preventing us from using it here?
There was a problem hiding this comment.
These insight models don't use any threshold to determine this state right now. We should probably be doing that to show between failing or informative.
Similar to #16272, this PR removes our copying of bf-cache/deprecation strings and instead plucks them from CDT.