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

Typescript Declarations #531

Closed
wants to merge 3 commits into from
Closed

Typescript Declarations #531

wants to merge 3 commits into from

Conversation

centrual
Copy link

@centrual centrual commented Jun 1, 2020

I can't find some method declarations in docs. This declaration file can be more clear if we can declare them.

Have a great day!

centrual added 2 commits June 1, 2020 23:14
(Fixed) event.target.value is not working.
(Added) Missing dropdown to templates. Also, types of templates changed from any to function.
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The typings should be first written to the native module and be used by another React wrapper typings. Most of the types I assume relates to the native module.

@yairEO
Copy link
Owner

yairEO commented Jun 6, 2020

What 😲😲😲
I have to maintain all this now?

Well, I guess this doesn't change much often, but I would have to remember to keep it updated, manually.

In what manner is this used?

@yairEO
Copy link
Owner

yairEO commented Jun 6, 2020

I see many parameters for methods are missing, why?

Also, I LOVE multi-typed function-arguments (love love...so much loving), so a parameter may be one of mutliple possible types, (mostly String/Array/Object) which the function itself is in-charge of understanding what to do accordingly.

Tagify has quite a few methods that can work with multiple types for some or the arguments (I reserve this rule for the first argument. other ones are mostly single-type)

This gives so much flexibility, for shorter & "forgiving" usage of Tagify

Does this file handles multiple-types for parameters?

@ghost
Copy link

ghost commented Jun 6, 2020

This is for autocomplete in VSCode

@ghost
Copy link

ghost commented Jun 6, 2020

See the GIF in this PR I opened in fiverr/passable to get an idea -> fiverr/passable#106

@sakethsaxena
Copy link

Any ETA on when this will be merged?

@yairEO
Copy link
Owner

yairEO commented Jul 13, 2020

I am reluctant to merge this for two reasons:

  1. it will inflate the package's size (people judge packages by this parameter)
  2. I would have to constantly maintain this file as Tagify evolves. It is a manual-time-consuming labour and if forgotten, I don't see how is it possible to know what was forgotten exactly (say, after a month) since there are so so many declarations
  3. Typescript is not mainstream, so most developers won't benefit from this extra (constant) work

@hugomn
Copy link

hugomn commented Aug 18, 2020

+1 for needing the typescript type definitions. If the package creator doesn't want to merge, can we create it at DefinitelyTyped at least? Thanks!

@yairEO
Copy link
Owner

yairEO commented Aug 22, 2020

@hugomn - what is DefinitelyTyped?

I don't want to merge because I would have to manually remember to manage this huge file for every change I make in the source code and it will bloat the NPM package, which will for certain deter people from installing Tagify, as they will not understand Tagify source-code by itself is not the cause for the large package-size, as typescript is a niche and not a must

@hugomn
Copy link

hugomn commented Aug 22, 2020

@hugomn - what is DefinitelyTyped?

I don't want to merge because I would have to manually remember to manage this huge file for every change I make in the source code and it will bloat the NPM package, which will for certain deter people from installing Tagify, as they will not understand Tagify source-code by itself is not the cause for the large package-size, as typescript is a niche and not a must

@yairEO thats not true. When you add type definitions to a package, it won't increase the module size after built when someone doesn't use ts. And tbh, I don't think the typescript adoption is that low as you think. You could count on the community to update the types.

DenitelyTyped is a separate repo where you could add the difinitions there and people would install them from another npm package.

@yairEO
Copy link
Owner

yairEO commented Aug 22, 2020

@hugomn -This number below is a big concern of mine, I am trying as low as possible.

image

I cannot 100% rely on the community to keep the declaration file it up-to-date because Tagify's source is quite large and code changes might be easily overlooked, might might create problems for other developers relying on out-dated declarations.

@kurtinaitis
Copy link

Who can push it to DefinitelyTyped?

@yairEO
Copy link
Owner

yairEO commented Sep 30, 2020

@centrual - could you make a PR for this in DefinitelyTyped like others have suggested in this discussion?

https://github.com/DefinitelyTyped/DefinitelyTyped#how-can-i-contribute

@jpbberry
Copy link

jpbberry commented Jan 4, 2021

Any progress on this? Without any type declaration the package essentially becomes useless in typescript.

@jpbberry
Copy link

jpbberry commented Jan 4, 2021

I'd even think that something like would benefit from being coded directly in typescript, you should try it out @yairEO
It would 100% be beneficial to compatibility and could possible cut down on unpacked size too, you wouldn't need a lot of the polyfills etc, and would handle the single file compiling for you and provide the declaration files, meaning you wouldn't need to maintain one big file of typings and rather you just need to write you code with types and all will work beautifully.

@yairEO
Copy link
Owner

yairEO commented Jan 12, 2021

@jpbberry - why is it useless? it's a third-party script like countless others on the internet. A project based on typescript can easily integrate it. it's not "useless".

I am just being lazy. As I said in my previous comment, somebody should take initiative and contribute declarations to:

DefinitelyTyped/DefinitelyTyped#how-can-i-contribute

I just don't want to bother myself with any typescript and keeping it up to date.
I am flooded with other open-source projects I want to advance with, and wish to invest minimum time on Tagify, as it is very stable and working well.

@jpbberry
Copy link

jpbberry commented Jan 12, 2021

That's fine I guess I'll wait for 3rd party types.

I called it useless because of the way typescript in itself works. I get absolutely no codesense and especially with a mundane addition like this one that's only gonna be used one or two times.
I have to memorize every single method and what they do, and what parameters they take and a project like this where you're not mainstream and don't have built in codesense makes it extremely difficult to use in anything that isn't a browser console that automatically runs through all the properties, which is just not what it should be like.

I'll probably just end up building something myself but types would make the biggest difference imo.
@yairEO

@yairEO
Copy link
Owner

yairEO commented Jan 12, 2021

@jpbberry - what exactly do you need to memorize? how is this different from any other API of a third-party script you must "memorize" when importing to a project? You cannot possibly re-build yourself countless third-party scripts which some took years to create, just so you could have autocomplete in your IDE...

simply go to the README of third-party code and read the docs when you need to work with it, which is probably not a thing anyone would do on a daily basis. Once you set up Tagify, it's pretty much done I would say, and it's not that hard..there are already many examples in the demo page for many of the possible scenarios, so I don't understand why would you want to waste months of your time re-build a tags component, only to have typescript declarations...

@yairEO
Copy link
Owner

yairEO commented Jan 12, 2021

Anyway, the file in this pull request is for React, for some reason, and I would prefer it not to mention React at all, since this library is not necessarily for React

@jpbberry
Copy link

I don't disagree with the react part, I wasn't planning on using it for react, however importing the library with something like webpack should be supported, there's no reason not to.
Typescript begins to remove codesense when using an interop module like this one, for example in js
image
in typescript
image
typescript would be the exact thing to be supported if this package were to exist on npm at all, else why wouldn't someone just use a CDN and <script> a js file, it seems useless to support node packages, unless you're using typescript, something that's all too commonly built and webpacked be it react or many other custom implementations.

I will say my initial reaction saying the package was useless was a little harsh so I am sorry for that, it just sucks to be left with no support for the language I use and many others would use this package for.
@yairEO

@gravitymedianet
Copy link

I can't find some method declarations in docs. This declaration file can be more clear if we can declare them.

Have a great day!

@centrual Can you please provide some comments on how to implement this?

@Brakebein
Copy link

I am just being lazy. As I said in my previous comment, somebody should take initiative and contribute declarations to:

DefinitelyTyped/DefinitelyTyped#how-can-i-contribute

I already made some type declarations as part of my Angular wrapper (ngx-tagify). So, I thought I can contribute them to DefinitelyTyped. I spent some time on working out how things are done (it's my first DT contribution), updating and completing the declarations (looking at both code and readme docs), and writing a test file (required by DT). Now, it's in the pipeline (see pull request) and will hopefully be available soon.

@yairEO
Copy link
Owner

yairEO commented Jan 30, 2021

BTW, you can simply import Tagify and get rid of all the annoying typescript issues:

microsoft/TypeScript#3019 (comment)

@Brakebein
Copy link

Brakebein commented Jan 30, 2021

Now, the type declarations are online (be aware of the 2 underscores):

npm install --save-dev @types/yaireo__tagify

I also exposed two interfaces TagData and TagifySettings.

// use interfaces this way
import Tagify from '@yaireo/tagify';

const settings: Tagify.TagifySettings = { delimiter: ',' };
const tags: Tagify.TagData[] = [{ value: 'hello' }, { value: 'world', class: 'red' }];

// or this way
import Tagify, { TagData, TagifySettings } from '@yaireo/tagify';

const settings: TagifySettings = { delimiter: ',' };
const tags: TagData[] = [{ value: 'hello' }, { value: 'world', class: 'red' }];

// ----
const tagify: Tagify = new Tagify(inputEl, settings);
tagify.addTags(tags);

I will try to keep the declarations up to date, if something changes in this library. But everybody is welcome to contribute.

TypeScript users might be happy now, as well as pure JavaScript users who want to use autocomplete in their IDE. Happy coding!

Edit: You might need to set "allowSyntheticDefaultImports": true in your tsconfig.json.

@centrual centrual closed this by deleting the head repository Dec 28, 2022
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.

8 participants