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

add type script declaration #33

Closed
wants to merge 1 commit into from
Closed

add type script declaration #33

wants to merge 1 commit into from

Conversation

taoqf
Copy link

@taoqf taoqf commented Dec 7, 2020

I am new to ulog and find this is a excellent lib, I want to help with typescript. this will work for all example in readme.

@Download
Copy link
Owner

Download commented Dec 7, 2020

That's great taoqf! Thank you very much for your interest and willingness to help!!

I am always looking for help, because I will never be able to build the ultimate logger alone. And you will be able to help shape what the solutions look like. I have been working on ulog v2 for a long time now, and it changed a lot between v1 and the early betas, but currently I am at a point where I feel the basic design has stabilized and other people can come on board and contribute, so if you are willing to do so then thank you!

About your PR. Can you give me some instructions on how to test this?

And how exactly did you make this? Did you copy-paste some code perhaps? I am asking because I see this:

assert(...args: any[]): void
dir(...args: any[]): void
table(...args: any[]): void
time(...args: any[]): void
timeEnd(...args: any[]): void
verbose(message?: any, ...args: any[]): void
silly(message?: any, ...args: any[]): void

These methods don't actually exist on all loggers.

Also, 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?

So maybe we should start simpler 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) { /* ... */ }

I can imagine we could maybe make a typescript definition for all standard entries. What I mean by that is that I am planning on providing a bunch of standard configurations of mods:

  • 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)

I have to admit that this is not fully thought out yet. I am still experimenting. So maybe for now we should focus on the ulog entry. I cam imagine we make a typescript definition that has all the properties and methods you get on ulog and all loggers when you require('ulog').

@jirutka
Copy link
Collaborator

jirutka commented Dec 7, 2020

The type declarations are copied from my PR #21, but without preserving authorship! @taoqf has just copied them and committed under his name. This is very disrespectful.

@Download
Copy link
Owner

Download commented Dec 7, 2020

@jirutka Thanks for the heads up!
If you have read my comments here, do you know of any way to dynamically extend a type definition based on which mods are in use? My gut feeling says it's not possible as I think typescript works at build-time right? So then our best option would be to add typescript definitions for the standard entries we provide such as 'ulog', 'ulog/full', 'ulog/debug' etc? If you agree, do you happen to know how we would go about this?

@taoqf Thank you for wanting to help out, but I need you to respect authorship. If Jakub did the work, he should get credit for it. So please keep this in mind when creating PRs.

@taoqf taoqf closed this Dec 7, 2020
}

declare namespace Logger {
function use(options: Partial<Middleware>): void;
Copy link
Author

Choose a reason for hiding this comment

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

Here, I add a method to Logger. you can add mods as well:

declare const mods: unknown[];

@taoqf
Copy link
Author

taoqf commented Dec 7, 2020

I use some code of @jirutka 's.
add finished tests code in types/test.ts
I will close this pr if @jirutka minds.

@Download, I am not very familiar with the lib, just add some typescript based on @jirutka 's pr.
It's not a correct type declaration but this is a way that you can add typescript declaration files to your package.
If you want check the types, just put your usage into types/test.ts, the definition will look good If typescript shows no error in your ide(or run tsc -p ./types/tsconfig.json as well, tsc is alias name of typescript which you probably install typescript use npm first.)

@Download
Copy link
Owner

Download commented Dec 9, 2020

@taoqf @jirutka
I opened a new issue #37 for typescript support.

If you both agree, we could maybe make a 'community effort' PR and you both will be credited along with anyone else that steps in to help. Adding the tests is very worthwhile @taoqf . In essence, you did nothing wrong by copy-pasting code; that's why we do Open Source to begin with, so we can use each other's work. Only thing is that most of us want to be credited for the work. So if we sort that out we should be good to go I hope. Do you both agree?

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.

3 participants