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

Sort out typescript support #37

Open
Download opened this issue Dec 9, 2020 · 18 comments
Open

Sort out typescript support #37

Download opened this issue Dec 9, 2020 · 18 comments

Comments

@Download
Copy link
Owner

Download commented Dec 9, 2020

I would like ulog to have typescript support out of the box.

This has been requested multiple times:

Now is the time to do it as the library design for v2 has mostly stabilized now and the public signature for ulog is mostly clear.
There are some challenges to overcome though.

We have the complex situation that methods and properties are being added to ulog and all loggers dynamically at runtime through mods. E.g. in normal situations (using the default mods), two methods, get() and set(), will be added to ulog by the settings mod. In fact any mod that has an extends key will end up extending the ulog object itself. Looking at the source of the settings mod for example we see this:

module.exports = {
  extend: {
    settings: {},
    get: function() { /* ... */  },
    set: function() { /* ... */ },
  }
}

So this mod adds a settings property and methods get and set to ulog.

I have no idea how to model this with Typescript... Maybe mods could include their own typescript definitions? But I think we will find that it's not actually possible because Typescript AFAIK is a compile-time tool so cannot rely on run-time behavior.

So for now my idea is this:

We start simple and first make the base typescript definition that extends the one from anylogger and only adds the stuff that is added in core:

ulog.mods = []
ulog.use = function(mod) { /* ... */ }

Let's call this core.d.ts

We then make typescript definitions for all the standard entries that ulog offers:

  • ulog: The basic config you get when you require('ulog')
  • full: The config with all mods included you get when you require('ulog/full')
  • debug: A config designed to closely resemble debug
  • ... (not sure yet)

So we would get:

  • ulog.d.ts extends core.d.ts
  • full.d.ts extends ulog.d.ts
  • debug.d.ts extends ulog.d.ts
  • ...

I would love for someone that is experienced with Typescript to prepare a PR for this. This time, it will get merged, I promise you!

Here is a list of of people that have expressed an interest in adding typescript to ulog in the past:

I am hoping one of you finds the time to help out with this. I know you wanted this in the past, but the project (read: I) wasn't ready for it yet. Now, we are ready to add this, so grab your chance and get listed on the credits! 👍

@jirutka
Copy link
Collaborator

jirutka commented Dec 9, 2020

I already wrote core types a year ago in #21, which I'm using in my projects, I just didn't write tests for them. I will try to get back to it and propose how to deal with extensions later.

EDIT: I'm just reading the sources and I see that you have changed it quite a lot.

@Download
Copy link
Owner Author

Download commented Dec 9, 2020

Yeah I will, if no one else takes the lead, probably end up preparing a PR myself based on your work and the tests of @taoqf
I think the approach using specific Typescript definitions for different entries will be good enough for most users. Also I think your definition already extends the one from anylogger right? So I can probably use the same trick to make e.g. ulog.d.ts extend core.d.ts etc. I am actually really motivated to get back to work on this but I have some other (paid) work I have to do first so it will probably not happen before next week.

On an unrelated note: I actually found a way to have pretty advanced log formatting without mangling the call stack and I'm thrilled about it. It would mean we would get debug-style formatted messages, but with working filenames/line numbers in the browser console, unlike debug's that are mangled. It would be a USP for this library I think. Hoping to create a new beta with that functionality in it very soon!

EDIT: just saw your question. Yes, anylogger is still very relevant. In fact, the default export of ulog is just the anylogger factory function, enhanced. And yes, the sructure of this library changed a lot from a year ago. And in fact, a year ago, I was already working on that change. That's why I didn't just merge pull requests like yours, because I knew it was based on a version of ulog that was about to radically change. I'm very sorry it took me so long to flesh it out, but yeah, you know, life happened etc. Corona has actually been good for my productivity on ulog :) I am very happy with the outcome. I feel ulog now is very modular and extensible and I managed to get most features that I wanted in, without growing the lib too much. It's still smaller than debug :)

@taoqf
Copy link

taoqf commented Dec 9, 2020

@Download I am sorry I do not have much time on this either.
This is the way to extends static members: use and `mods':

core.d.ts

import { BaseLevels, BaseLogger } from 'anylogger'

interface Logger extends BaseLogger {
	...
}

declare namespace Logger {
	function use(options: Partial<Middleware>): void;
	var mods: unknown[];
}

export default Logger;

@Download
Copy link
Owner Author

Download commented Dec 9, 2020

@taoqf Thanks! All that stuff is helpful and will help me compile a PR myself eventually if no one has the time to step in. 👍

@Vadorequest
Copy link

You might be looking for TS Generics. https://www.typescriptlang.org/docs/handbook/generics.html

It's definitely possible to allow a dynamic implementation of get/set. The base library would define generic types that can be overloaded by the lib that actually implement the method.

@Download
Copy link
Owner Author

Download commented Dec 9, 2020

@Vadorequest Thanks that sounds good!!

@Vadorequest
Copy link

Also, this might not be what you want, but it's worth asking.

Did you consider writing ulog in TypeScript? You wouldn't have to write the types separately, and will likely avoid many "out-of-sync" issues between JS and TS in the future.

@Download
Copy link
Owner Author

Well yes it might be a good idea to port it over to typescript one day. But I have to learn Typescript first. But for now I want to finish v2.0.0. And I'd like that version to have some 'correct' typescript support. And yes it going out of synch could definitely be an issue. So at the very least I should learn how to test whether the typescript definitions are 'working' so I can check that before I release. So some tips on that would be great as well.

@Vadorequest
Copy link

The first thought coming to my mind about "testing the type definitions" would be to write a few unit tests using TS. Either on a different repository, on directly on this one.

That'd likely be enough to test the types. And that could be reused when porting the base code to TS too.

@jirutka
Copy link
Collaborator

jirutka commented Dec 11, 2020

Sorry, I’m extremely busy now, but I will try to find some time this weekend.

@Download
Copy link
Owner Author

Download commented Jan 23, 2021

Update: I'm learning Typescript. As an exercise I have added Typescript definitions to project kurly, which is a dependency ulog has. I am testing it by compiling a test.ts file with tsc. It seems to be working well. So I think that's a good approach for ulog as well. But I do have some issues with getting exports and imports right. Not a Typescript thing per se but it's an extra confusion. Atm, when using Kurly, one needs to have esModuleInterop: true in the tsconfig.json... Ideally this flag should not be needed. A related discussion is going on in the anylogger repo: Download/anylogger#15

@Download Download added this to the v2 milestone Feb 4, 2021
@jirutka
Copy link
Collaborator

jirutka commented Feb 8, 2021

I’ve spent the last five hours trying to understand ulog modules, how they interoperate, where are various properties created, reverse engineering what types of arguments the functions accept… and writing type declarations. I think that I more or less understand it now. I made a PoC with several modules (all except config and formats where I gave up) and I think that it can be finished, but I don’t think it’s worth it anymore. I’m completely exhausted and disgusted.
The code is modular ad absurdum which makes it awfuly complicated and unclear, messy in some parts. It’s extremely hard to read and reason about; I couldn’t even imagine hunting bugs in it. And the worst is that I don’t see any real benefits of such design that would balance this complexity; it just looks overengineered as hell for me.
I admire you for being able to design and code such monstrosity, but I really don’t wanna use it anymore nor contribute to this project. If I can give you an advice, start coding in TypeScript (in strict mode; and not just type declarations), it will hopefuly limit your creativity a bit in a good way.

@toddb
Copy link

toddb commented Apr 27, 2021

@jirutka I am going to keep using it because I am too deep in. However, agreed with the rest around understandability (and being too clever).

@Download I would add that you are seeing other incompatibilities (eg #63) that you don't need to solve with the right build chain set (eg using browerlist). I think the advice above should be taken seriously if you want ulog and anylogger to be the go to solution for professionals.

Also, the marketing (the smallest) is good but actually your selling point needs to not be small but concise, working and for most professionals readable, documented and understandable code.

:-)

@Download
Copy link
Owner Author

Download commented May 5, 2021

Mmm trying to be too clever.... Sounds like me yes. Sorry.
Harsh words. Painful to hear. But I need to hear it.
@jirutka Sorry to hear you give up. Do you maybe have an actionable tip for me?
Like how should I redesign stuff so it would be more acceptable to you?

@toddb
Copy link

toddb commented May 5, 2021

@Download It really depends on your goals. If you want to be the "goto" for logging in this space, then this probably isn't a technical question. It's a customer question. Are you getting the adoption you want? If not, find out why and act on that (but the warning here is that product work is hard—balancing own vision and listening!)

I thought @jirutka's point was that the logging proposition is good but because it is a library it needs good design throughout—and that particularly includes dependency management. For typescript users (who I personally think you are wrong not to target), there is not enough API design in practice. @jirutka then pointed out that internal design is not readable/understandable either.

I hope there is something useful in there to inspire!

@Vadorequest
Copy link

I can only recommend TypeScript. While a bit complicated to setup, it clearly pays off when being used. Everything is simpler to understand.

@Download
Copy link
Owner Author

Download commented May 5, 2021

@jirutka Well, the good news is that in my new job, Typescript is extensively used. :)

@Download
Copy link
Owner Author

Download commented May 5, 2021

@toddb Oh I do wish to target Typescript, but I don't know how. I'm new to Typescript, so I don't know where the pain is. What I need is some hands-on experience. I'll get that in my new job. But it will take some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants