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

refactor(kit,nuxt,schema): use consola and improve test dx #23302

Merged
merged 33 commits into from Sep 19, 2023

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Sep 19, 2023

πŸ”— Linked issue

Extracted from #23225

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR refactors all packages to leverage consola logger exported from @nuxt/kit.

We use a magical conosla wrapper in CLI that was automatically wrapping all console usages but it can be easily missed when running in tests or with programmatic usage.

Exceptions:

  • For tests, we add a guard of process.client to avoid clattering output
  • For schema package, we directly use consola to avoid kit dependency
  • For nuxt/app and other runtime code, we use console for bundle size

Added back a (warning level) eslint rule also for no-console

Note: This PR also contains small other eslint tweaks to remove all current warnings.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@stackblitz
Copy link

stackblitz bot commented Sep 19, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@pi0 pi0 changed the title chore: improve tests and logging dx refactor: use consola logger from kit and improve tests dx Sep 19, 2023
test/setup-env.ts Dismissed Show dismissed Hide dismissed
@pi0
Copy link
Member Author

pi0 commented Sep 19, 2023

TLDR: output is more stable yet, Logs are still somehow leaking and spamming CI/Test output. Also, we have random failures with Ubuntu preset (most likely concurrency) but i gues it is worth to iterate over such big refactor into smaller steps...

@pi0 pi0 requested a review from danielroe September 19, 2023 19:17
@pi0 pi0 marked this pull request as ready for review September 19, 2023 19:17
@pi0 pi0 mentioned this pull request Sep 19, 2023
8 tasks
try {
requests.push(req.url().replace(url('/'), '/'))
} catch (err) {
// TODO
Copy link
Member

Choose a reason for hiding this comment

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

what kind of error was being triggered here?

Copy link
Member Author

@pi0 pi0 Sep 19, 2023

Choose a reason for hiding this comment

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

The lovely context is not available one by late calling url()! Turns out we somehow still keep loading the Browser pages after tests are being done and we don't have context anymore.

This was more visible in new nuxi-edge case because we early close server properly now and made it more visible.

However i think would be nice later investigate root causes. Seems we are wasting too much on browser allocation in our tests.

Copy link
Member

Choose a reason for hiding this comment

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

nice find!

@danielroe danielroe changed the title refactor: use consola logger from kit and improve tests dx refactor(kit,nuxt,schema): use consola and improve test dx Sep 19, 2023
@danielroe danielroe merged commit 2bf9028 into main Sep 19, 2023
25 checks passed
@danielroe danielroe deleted the chore/improve-loggin branch September 19, 2023 21:26
This was referenced Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants