Download / ulog

The universal logger
https://ulog.js.org
MIT License
88 stars 19 forks source link

No logging message appear (ulog doesn't look to be loaded) #66

Open toddb opened 3 years ago

toddb commented 3 years ago

The documentation for running anylogger assumes anylogger as bundled package where it is directly loaded in the browser and globally available. The problem looks to be that ulog is not actually bundled because the only reference to ulog as the implementation is an import. Bundlers will treeshake the package.

Expected

Add logging to a project and logging messages will be written to the console in the browser

Actual

Nothing appears inn the browser console.

Steps to reproduce

Created a brand new Vue project and add logging

  1. vue create logger-demo
  2. yarn install anylogger ulog
  3. add the logging code into main.js
  4. yarn serve
  5. load up the browser, dev tools and look at messages

Based on the sample README, only a reference to anylogger is required.

import anylogger from 'anylogger';

const log = anylogger('Logging');
log.debug('I will not log unless log level is reduced');
log.error('I should log even in default settings');

// result nothing logs

Add ulog import, still doesn't work (under my build system-let's assume "tree shaking" broadly)

import anylogger from 'anylogger';
import ulog from 'ulog';

const log = anylogger('Logging');
log.debug('I will not log unless log level is reduced');
log.error('I should log even in default settings');

// result still nothing logs (as ulog still isn't there)

Of course, the logging library is actually required to implement the interface. Without typings, it all gets hard and picked something random to log that ensures the library is bundled.

import anylogger from 'anylogger';
import ulog from 'ulog';

const log = anylogger('Logging');
// need a reference to include library and ensure no linting errors
log.debug(ulog.levels);  // ah, need typings so added `declare module 'ulog'` (or could use `ulog.d.ts`)
log.debug('I will not log unless log level is reduced');
log.error('I should log even in default settings');

// now I log
Download commented 3 years ago

Hi @toddb have a look at this section of the documentation:

Add to entry point

In the entry point of your application import ulog:

index.js

import "ulog"

https://github.com/Download/ulog#add-to-entry-point

You should import ulog (or some other anylogger adapter) as the very first import. That will ensure it is loaded first and then it imports anylogger and overwrites anylogger.ext, making sure that all created loggers will be ulog loggers. If you have a project on Github or somewhere else where I can see it than I can take a look at it to hopefully get it working for you.

I am open to ideas to improve the documentation on this. I kinda have a blind spot here since I know exactly how it works so I am not sure what I should change to make it more clear.

EDIT:

log.debug(ulog.levels); // ah, need typings so added declare module 'ulog' (or could use ulog.d.ts)

Mmm yes.... I am not sure about this part. I am a typescript novice. Does whether or not you use typings have any influence on tree-shaking? I can imagine that the statement import "ulog" is assumed to have side-effects and so never tree-shaked, whereas import ulog from "ulog" is maybe expected to be tree-shakeable if you never actually use the imported ulog object?

toddb commented 3 years ago

All your examples are without build chain tools. You just need to be aware of this. IMHO, you are a library builder of a library that is central (to real/professional software) that these days is just about never included as you document. We also use tool chains—thus an import is not enough. Just make a note of this is a bare minimum.

Note: this occurs because you auto register ulog. One way around this is to document manual registration which means that tool chains include the library. I personally did it by using it in a log message.

This is a tiny point. But it questions if the tool builder fully understands the customer and the customers' scenarios :-)

Download commented 3 years ago

All your examples are without build chain tools.

I am using Webpack myself in the build of ulog. Check the webpack config. Show your code that is not working.

Note: this occurs because you auto register ulog. One way around this is to document manual registration

Which is what the docs about adding to the entry point is supposed to be, If it's not good, please elaborate on how to improve it.

it questions if the tool builder fully understands the customer and the customers' scenarios :-)

So show me your scenario. How can I understand your Typescript scenario if I'm not a regular typescript user and you don't show it? ulog works find i.c.w. build tools. I specifically wrote it so it would work great with that.

nopmop commented 3 years ago

You could say: because ulog auto registers, importing it at the beginning of each entry point of your app can end up not working with complex Webpack/Typescript compiler setups. An alternative then can be to use webpack's ProvidePlugin in the plugins section of your webpack.config.js like so:

        new webpack.ProvidePlugin({
          // ...
          ulog: 'ulog',
        }),

and then write if(ulog) true at the beginning of each module of your app where you want ulog to be available. Alternatively, if you provide also anylogger via ProvidePlugin, you can implicitly trigger auto registration and assign the logger like so: var log = ulog ? anylogger('myModule') : () => {}

Also say: NOTE: the default level for logging in the browser is set to WARN. If you want to change it, say to ALL, and you encounter some type errors while using Typescript, you can use this syntactic workaround: log['level'] = (log as any).ALL

@Download perhaps the README focuses a bit too much on discussing how you took so many awesome features from other packages, like when you quote the code from other loggers (DEBUG=test,my:*,-my:lib) a distracted user might be tempted to try that without realizing that it's a configuration option for debug. Actually the configuration via environment variables is unclear to me as well: with webpack running it with log=*=debug webpack ... or log=debug webpack ... or webpack --env log=debug doesn't seem to work. Oddities... Where is this environment taken from? process.env in NodeJS? I'll have to read the code...

My two cents.

Download commented 3 years ago

because ulog auto registers, importing it at the beginning of each entry point of your app can end up not working with complex Webpack/Typescript compiler setups.

I still fail to see which 'complex setups' have an issue. Also, I don't really think ProvidePlugin is the answer. But I haven't actually seen the setup you are using that you have prolems with, so I may be wrong. I would still be very interested in seeing (and cloning, testing) the actual setup you have. Maybe you can share a repo for me to look at?

@Download perhaps the README focuses a bit too much on discussing how you took so many awesome features from other packages, like when you quote the code from other loggers (DEBUG=test,my:*,-my:lib) a distracted user might be tempted to try that without realizing that it's a configuration option for debug.

Yes, this may be true. On the other hand debug is the most popular logging package on NPM right now so I want to show people that ulog can be a drop-in replacement.

I have been swamped with work the last few months so I'm not working on ulog at the moment but one day I will come back to this. In the mean time if you want you could make a PR for the docs and or share a repo for me to look at. I am uncomfortable writring tips related to Typescript in the README myself because I am too unfamiliar with it.

eviltik commented 3 years ago

Got an issue, probably related to that ticket ?

Not working (no log in the console) :

import ulog from 'ulog';
import anylogger from 'anylogger';

const log = anylogger('Logging');
log.info('yeah');

This works:

import ulog from 'ulog';
import anylogger from 'anylogger';

const log = anylogger('Logging');
setTimeout(() => {
    log.info('yeah');
}, 200);

Same thing everywhere (i.e import ulog in index.js and import anylogger in another file)

Download commented 3 years ago

@eviltik I guess you are including ulog.min.js via a script tag? That will lazy-load ulog.lazy.min.js and this will give you a short delay in which logging is not yet available. Please try with ulog.full.min.js and see if it makes a difference for you

eviltik commented 3 years ago

@Download no, i've npm install, then i'm using webpack with imports. no lazy load.

Download commented 3 years ago

Is there something you can share, like a Github project, that I can test out?

eviltik commented 3 years ago

yep, let me create a simple case

eviltik commented 3 years ago

here it is : https://github.com/eviltik/ulog-test

eviltik commented 3 years ago

main code (src/frontend/index.js)

import ulog from 'ulog';
import anylogger from 'anylogger';

const log = anylogger('my app');

localStorage.setItem( 'log', 'info' );

// This fail
log.info('immediate');

setTimeout(() => {
    // This is ok
    log.info('delayed 1sec');
}, 1000);

"immediate" not written into the console as it should "delayed 1sec" written into the console as expected.

Thank you

toddb commented 3 years ago

@eviltik here's my code inside a Vue application registration. These logs do get written to the console. There are some very subtle differences and I wonder if that makes a difference (which are the subject of this message). You could try logging 'ulog' to see if you see the same behaviour. (sorry, I see your repo but struggling with time at the moment.)

import anylogger from 'anylogger';
import ulog from 'ulog';

const log = anylogger('app');
log.debug(ulog.levels);
log.debug('Set log level \'%s\'', process.env.NODE_ENV);
toddb commented 3 years ago

@eviltik I also noticed this bug https://github.com/Download/ulog/issues/56

eviltik commented 3 years ago

yes @toddb i noticed this bug too.

@Download what do you think ? thank you

toddb commented 3 years ago

@eviltik I think @Download has put out his design—ulog will just do its thing to "work". It's just not meeting all needs unfortunately.

I'm seeing others ask for async loading, you are looking for sync, I am looking for explicit. :-)

Download commented 3 years ago

Sorry I have been very busy. I will check out your code base later.

We will get this fixed.

@toddb I am open to suggestions for changes / improvements, but please make them actionable. And in the past, some have said things like 'just rewrite this in typescript', which I don't think is a reasonable change to suggest. I do like Typescript and foresee a version 3 written in Typescript in the future, but I intend to finish v2 first, which means solving all the little issues and making sure this is a robust logging library that people can rely on to work. Given how long it took to get here, that may take a lot of time still.

The current design is working pretty well imho and at this stage I intend to keep it like this and just iron out the issues first so we all have a stable 2.0 to work with. My design for this library is trying to meet some very ambitious requirements, many of which are actually contradictory:

My top priority is this great feature set at a microscopically small size. And if I have to write the code in Chinese or binary to get it, I will write the code in Chinese or binary. Writing readable code is great, but having a tiny and rich logging library is more important to me. Sometimes the design may seem contrived (and I'm sure it can be improved), but if these improvements make the library bigger without adding features, I'm against them. Because in my experience. only a few devs actually read the code. The fast majority just uses it and they could care less on how it was written as long as it works great and doesn't bloat their bundle. And that's exactly what I intend ulog to do.

Download commented 3 years ago

@eviltik Took a quick glance at your codebase and it looks fine to me. Don't have the time to dig into it right now but I will. I promise.

Just a heads-up that you cannot combine the lazy loading with the sync behavior that you seek. But afaict from the quick glance at your code, you are not trying to.

I do want sync loading to work. You won't get the 2.7kB footprint (it will be closer to 6kB) but that's life. We have to make hard choices sometimes. But just to let you know, I deeply care about the sync loading scenario to work and if the fact that it's not working at the moment in your code is due to a bug in ulog I will make sure it gets fixed before the 2.0 final release.

eviltik commented 3 years ago

thank you @Download

It's important to fix this bug asap, because since a few months, a user who is testing ulog with webpack for this kind of usage will drop your lib immediately. I'm at this point too right now. Waiting for your fix ! ty

harinair commented 2 years ago

+1 Exact same issue I am experiencing in my React project which is using a npm workspace and monorepo. Any chance this can be fixed?

harinair commented 2 years ago

Any work arounds? No one else facing this issue. I tried pretty much everything and I decided to remove ulog. My belief is it does not work in a rollup react library setup. Tried a lot of stuff... but I don't want to spend more time on this.

toddb commented 2 years ago

@harinair I see a couple of practical solutions. Roll back to ulog v1 (it did everything I wanted and mostly worked). I wouldn't recommend rolling your own Console anylogger implementation because IMHO it needs some logging level configuration that start to be a lot of work (ps I did :-().

I hope there is some sense in there that is useful.