fix: Extend mock logger used in angular bootstrap #25500
Conversation
* Add missing logger functions that caused failure * Proxy mock logger to debug so output isn't swallowed when troubleshooting * Minor test performance improvement
| it('should live-reload on src changes', () => { | ||
| cy.visitApp() | ||
| cy.contains('app.component.cy.ts').click() | ||
| cy.waitForSpecToFinish({ passCount: 1 }, 60000) |
There was a problem hiding this comment.
Not your change but woah,60000, does the spec really take that long to execute? 🐢
There was a problem hiding this comment.
The angular tests especially (and other frameworks to a lesser extent) take forever to bundle & launch webpack on Windows, so I think this was added to work around that.
| @@ -203,14 +205,23 @@ export async function getAngularJson (projectRoot: string): Promise<AngularJson> | |||
| return JSON.parse(angularJson) | |||
| } | |||
|
|
|||
| function buildLogger (name: string = ''): logging.Logger { | |||
There was a problem hiding this comment.
Seems this is only called once (below) and name is never passed, is that required, or can we remove the argument entirely?
There was a problem hiding this comment.
It's actually called in two places, one of which is within itself so it's easy to miss. We call it once with no name for the root logger, and the createChild signature accepts a name for child loggers
| function buildLogger (name: string = ''): logging.Logger { | ||
| const debugLogger = debugLib(`${debugPrefix}${name ? `:${name}` : ''}`) | ||
|
|
||
| return { |
There was a problem hiding this comment.
Cannot find any docs in Angular about the logger, nice job figuring this out.
Another option if we want to cover all bases might be using a Proxy and pointing everything that's not one of the properties you defined here as a no-op. Then if someone does something exotic like logger.subscribe() it wouldn't error (it also wouldn't do what they are expecting, though).
Will let you make the final call, just throwing another suggestion out there.
There was a problem hiding this comment.
Took another look based on your suggestion and I ended up finding a better alternative - just build an actual Logger instance (which is really just a tracker for LogEntry items) and build a subscription that forwards everything to the debug logger. No idea why we weren't just building a Logger instance to begin with, but now we're fully compliant with the contract and get inspectability from the debug logger
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
User facing changelog
Fixes an issue where Angular Component Testing projects would fail to initialize if an unsupported
browserslistentry was specified in the project configuration.Additional details
We were building an incomplete
loggerinstance for use by@angular/devkitutil functions - if an unsupportedbrowserslistentry was specified the utils would try to make a logging call that would fail. This PR makes a couple changes:as unknown as logger.Loggercast) but this should cover most use casesdebugso that it's available when troubleshooting - output would previously be swallowedSteps to test
npx @angular/cli new my-project.browserslistrcfile with the contentIE 11npx cypress open --componentyarn buildinnpm/webpack-dev-server, and open the project,. Observe no failureHow has the user experience changed?
Project with unsupported
browserslistconfig will now openPR Tasks
cypress-documentation?type definitions?