Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lets the watcher CLI show some logs in debug/trace and others all the time (and adds duration) #8591

Merged
merged 9 commits into from
Jun 28, 2023

Conversation

orta
Copy link
Contributor

@orta orta commented Jun 12, 2023

  • I copied the colour/style format of the functions CLI output,
  • Moved output like files created into something which is only available when LOG_LEVEL is trace
  • Logs the time it takes to generate types for any section
  • Traces when the fs message is received, and not just when something has happened

level: logLevel,
redact: redactionsList,
}
} satisfies LoggerOptions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new TS 4.9 feature, lets you say "this object must conform to this type" but doesn't force the type to be that type to other objects.

What this means is that I can access defaultLoggerOptions.level without a ? because it's not LoggerOptions's string | undefined but this object's string.


cliLogger.trace =
logLevel === 'trace' || logLevel === 'debug' ? console.log : () => {}
cliLogger.debug = logLevel === 'debug' ? console.log : () => {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function objects have special cases for Expandos which let them be lazily checked so you can append extra properties to the type

Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @orta, just tried to test this one out locally but ran into a few things and left a few suggestions

packages/internal/src/cliLogger.ts Outdated Show resolved Hide resolved
packages/internal/src/cliLogger.ts Outdated Show resolved Hide resolved
packages/internal/src/generate/watch.ts Outdated Show resolved Hide resolved
orta and others added 2 commits June 13, 2023 08:20
Thanks

Co-authored-by: Dominic Saadi <dominiceliassaadi@gmail.com>
@orta
Copy link
Contributor Author

orta commented Jun 13, 2023

Thanks! I've been having issues with rwfw project:sync by the end of my session and couldn't run it

@jtoar
Copy link
Contributor

jtoar commented Jun 13, 2023

@orta yeah we had a bug with it this past week that I think I finally tracked down here: #8579 (original issue here #8580)

Will get David to sanity check it again tomorrow but that should be fixed very soon

@jtoar
Copy link
Contributor

jtoar commented Jun 13, 2023

@orta just merged that PR that fixed project:sync. Very sure that it should be back to being solid now but let me know if anything comes up

Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good @orta but found one bug in the level config, let me know what you think

@jtoar jtoar added the release:feature This PR introduces a new feature label Jun 15, 2023
@orta
Copy link
Contributor Author

orta commented Jun 28, 2023

I got this running locally now, and agree - switched the fns around

@jtoar
Copy link
Contributor

jtoar commented Jun 28, 2023

Thanks @orta! Awesome I'll handle getting this merged

@jtoar jtoar merged commit aa14770 into redwoodjs:main Jun 28, 2023
9 checks passed
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Jun 28, 2023
@jtoar jtoar modified the milestones: next-release, v6.0.0 Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants