chore: refactor log function to class #20382
Conversation
|
Thanks for taking the time to open a PR!
|
|
|
||
| if (!(!_.isFunction(value) || !groupsOrTableRe.test(key))) { |
There was a problem hiding this comment.
I changed this part to _.isFunction(value) && groupsOrTableRe.test(key), because it's a bit hard to read with double negation.
|
|
||
| this.fireChangeEvent() |
There was a problem hiding this comment.
It has been changed to this.logManager.fireChangeEvent(this).
| @@ -613,25 +631,5 @@ function create (Cypress, cy, state, config) { | |||
| return log | |||
| } | |||
|
|
|||
| logFn._logs = logs | |||
sainthkh
requested review from
jennifer-shehane
and removed request for
a team
packages/driver/src/cypress/log.ts
Outdated
| // as fast as every 4ms | ||
| // @ts-ignore | ||
| log.fireChangeEvent = _.debounce(triggerStateChanged, 4) | ||
| const log = new Log(cy, state, config, logManager, options) |
There was a problem hiding this comment.
Are we able to pass logManager.fireChangeEvent instead of the entire class instance? Since the logManger stores all of the incoming logs, this feels heavy to each log instance.
packages/driver/src/cypress/log.ts
Outdated
|
|
||
| const triggerStateChanged = () => { | ||
| return trigger(log, 'command:log:changed') | ||
| export function create (Cypress, cy, state, config) { |
There was a problem hiding this comment.
any reason the logFn logic couldn't live in the LogManager ?
export function create (Cypress, cy, state, config) {
const logManager = new LogManager()
return logManager.createLog(cy, state, config, logManager, options)
}
There was a problem hiding this comment.
I left it there because it was there for years.
After your comment, I also thought about it and it's Manager's job to create logFn. So, I moved it.
sainthkh
force-pushed
the
refactor-log-to-class
branch
from
2d0358e
to
ce95894
Compare
User facing changelog
N/A. It's an internal refactoring
Additional details
Summary of changes.
As it's a refactoring, the code change is hard to read. So, I left this.
LogUtils,LogManager,Log.LogUtilsare functions used in other files. They were exported withexport default. I could remove them.LogManager: Insidecreatefunction,logsvariable exists and it manages logs created. But it makes us createfireChangeEventfunction insidecreatefunction and attach it toLog. So, I createdLogManagerclass andLogclass reference it.Log: It was a function that returns an object. I changed it to a class.How has the user experience changed?
N/A
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?