Move logger to common-universal
#691
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📦 Size Report
|
packages/app/vite.config.ts
Outdated
| @@ -28,7 +28,8 @@ export default defineConfig({ | |||
| // nodePolyfills needs to be first | |||
| nodePolyfills({ | |||
| // buffer is needed when connecting with a coinbase wallet installed on a phone | |||
| include: ['buffer'], | |||
| // util and path is needed for common-universal's logger | |||
| include: ['buffer', 'util', 'path'], | |||
There was a problem hiding this comment.
Do we really need these? I would prefer not to use node apis in browser
There was a problem hiding this comment.
Util is used for inspect to format objects, path isn't doing anything in browser. I tried to conditionally import a path depending if that's a browser or not, but nothing worked - logger would have to be async which we don't want to do
There was a problem hiding this comment.
Anyway using util polyfill breaks frontend tests, so probably we need to figure out something else
| @@ -38,7 +34,7 @@ export class Logger implements ILogger { | |||
| service: options.service, | |||
| tag: options.tag, | |||
| utc: options.utc ?? false, | |||
| cwd: options.cwd ?? process.cwd(), | |||
| cwd: options.cwd, | |||
There was a problem hiding this comment.
Is it possible to use process when not in browser?
There was a problem hiding this comment.
In browser we can, but it returns undefined which is not compatible with process.cwd() return type. The cwd parameter is optional in the options.
We use process.cwd() in PrettyLogger in common-node
There was a problem hiding this comment.
Before we were using node:path's join function after obtaining cwd, that's why I haven't done this in common-universal
Additionally we can swap
mspackage withpretty-ms.The differences are:
UnixTime.ONE_YEAR()changes to '1y'UnixTime(UnixTime.ONE_YEAR() - UnixTime.ONE_HOUR())changes to '364d 23h'What do you think about swapping
mspackage? It's used only inUnixTime