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 typings #86

Closed
andreivanea opened this issue Nov 2, 2016 · 43 comments
Closed

TypeScript typings #86

andreivanea opened this issue Nov 2, 2016 · 43 comments
Assignees
Labels
Feature Large feature update

Comments

@andreivanea
Copy link

Is there any chance you could add TypeScript typings?

@olifolkerd
Copy link
Owner

Certainly. I will have a look into it in the next couple of days

@olifolkerd olifolkerd self-assigned this Nov 2, 2016
@olifolkerd olifolkerd added the Enhancement Possible small update to an existing feature label Nov 2, 2016
@olifolkerd
Copy link
Owner

You will have to excuse me as i am not a TypeScript developer myself, but from reading the documentation, as i understand it, tabulator.js should require no specific typings itself.

The jQuery specific typings it needs, should be included in the jQuery and jQuery UI libraries pulled in when you build your project.

Could you be specific in what you are looking for in this enhancement?

As you probably know more than me on this subject i would be happy for you to make the changes and submit them for inclusion in the repo in a pull request.

@andreivanea
Copy link
Author

I've never built a complete typing myself and I have been using TypeScript for a couple of months or so. You are right, the typings needed from jQuery should be automatically picked up. But, from what I've seen in other typings from jQuery plugins, the API should still require a description.

Now, I don't know if this is still the case when calling the API through string commands, but I assume that the config object and functions, like those used by the formatter would benefit from the typings.

I'll try something out in the next few days and see what is picked up and what should be described.

@olifolkerd
Copy link
Owner

Hey,

Did you have any luck figuring out the Typings that were needed?

Cheers

Oli

@andreivanea
Copy link
Author

Hey, I'm glad you're interested in this. I've actually made some progress. I believe that I have now the complete description of the options. However, the options which allow values of multiple types (e.g. string and functions) must be extended to accept all those types. I have specified just the primitive types, for now.

There's progress but I'm lacking the free time, to be honest. I hope to make a PR sometime next week. Then you'll be able to check if something is missing or errorneously defined.

@olifolkerd olifolkerd added Feature Large feature update and removed Enhancement Possible small update to an existing feature labels Dec 24, 2016
@olifolkerd
Copy link
Owner

Hey @andreivanea ,

Is this still something you are interested in perusing?

Cheers

Oli

@andreivanea
Copy link
Author

Hey,

Yes, I think this is a cool project and having the typings for it will make it easier to work with.

That being said, my "free time problem" has worsen. I did look through the docs of the new version, to find out what's new and what's gone, but did not manage to update the typings that I already have. I can't and won't give another ETA but I will try to find ~10 minutes/day to complete it.

@olifolkerd
Copy link
Owner

Hey,

No pressure or rush, i was just curious if it was something you were still interested in,

Take as much time as you need.

Cheers

@andreivanea
Copy link
Author

andreivanea commented Nov 2, 2017

@olifolkerd I'm (very slowly) making some progress here. You can check it out here. This is still going to take some time and, because I'm a bit new to the whole "GitHub experience", during this whole process I might click once or twice on the wrong buttons...

@olifolkerd
Copy link
Owner

Hey @andreivanea

Looks like you are making quite a lot of progress there! great work!

@angeliski
Copy link
Contributor

Hey @olifolkerd and @andreivanea Any update about that?
I would like to use that in my projects with typescript.

@olifolkerd
Copy link
Owner

@angeliski

Im afraid this was closed due to inactivity.

I would be happy to accept a pull request if you wanted to contribute these to the project.

Cheers

Oli :)

@angeliski
Copy link
Contributor

Nice @olifolkerd I will try work in that

@Jojoshua
Copy link
Contributor

Jojoshua commented Mar 6, 2019

@olifolkerd can you reopen this issue. I've been working on this.

@olifolkerd olifolkerd reopened this Mar 6, 2019
@andreivanea
Copy link
Author

I'm still interested in this but my spare time has been dramatically reduced lately. I'd be more than happy to take a look if someone starts implementing it. I'm assuming it will be valid for v4.

@Jojoshua
Copy link
Contributor

Jojoshua commented Mar 7, 2019

@olifolkerd I've completed the first revision of the declarations!
These need to be on DefinitelyTyped since Tabulator is not built in Typescript. Please review the index.d.ts file from my pending PR DefinitelyTyped/DefinitelyTyped#33652. I think you will find the way I typed this maps pretty well to the overall design/architecture of the working software but having your review would be great to validate things I missed.

@andreivanea @angeliski or anyone else using this library with Typescript -- it would be great if you could test your code against these definitions for further validation.

This was pretty intensive to create and required me to dig though lots of the code/docs. I have compiled notes as I was going through. Some relate directly to types, some relate to documentation on the website. The website docs were very helpful in this process.

--Docs
http://tabulator.info/docs/4.2/edit#edit-builtin
-autocomplete editor params freetext should probably be cased as freeText

http://tabulator.info/docs/4.2/options
-resizableColumns Explore link is broken.
-ajaxLoader,ajaxLoaderLoading,ajaxLoaderError could use an Explore button for details
The following are missing as available options:
autoResize
columnHeaderSortMulti
clipboardCopyStyled
clipboardCopyConfig
clipboardCopied
clipboardPasted
dataTreeRowExpanded
dataTreeRowCollapsed
selectableRangeMode
movableRowsSendingStart
movableRowsSentFailed
movableRowsSendingStop
movableRowsReceivingStart
movableRowsReceived
movableRowsReceivedFailed
movableRowsReceivingStop
tableBuilding
tableBuilt
renderStarted
renderComplete
rowClick
rowDblClick
rowContext
rowTap
rowDblTap
rowTapHold
rowMouseEnter
rowMouseLeave
rowMouseOver
rowMouseOut
rowMouseMove
rowAdded
rowDeleted
rowMoved
rowUpdated
rowSelectionChanged
rowSelected
rowDeselected
rowResized
cellClick
cellDblClick
cellContext
cellTap
cellDblTap
cellTapHold
cellMouseEnter
cellMouseLeave
cellMouseOver
cellMouseOut
cellMouseMove
cellEditing
cellEdited
cellEditCancelled
columnMoved
columnResized
columnTitleChanged
columnVisibilityChanged
htmlImporting
htmlImported
dataLoading
dataLoaded
dataEdited
ajaxRequesting
ajaxResponse
ajaxError
dataFiltering
dataFiltered
dataSorting
dataSorted
dataGrouping
dataGrouped
groupVisibilityChanged
groupClick
groupDblClick
groupContext
groupTap
groupDblTap
groupTapHold
pageLoaded
localized
validationFailed

http://tabulator.info/docs/4.2/page#manage
-previousPage copy/paste error in description

http://tabulator.info/docs/4.2/localize#setup

  • update getLang to account for locale parameter

http://tabulator.info/docs/4.2/columns#visibility
-showColumn,hideColumn,toggleColumn docs need to mention that it can take a column loookup rather than just the field name

http://tabulator.info/docs/4.2/columns#get-definition
-getColumn should mention it can take a column lookup

http://tabulator.info/docs/4.2/tree#layout-colexp
-Can dataTreeElementColumn,dataTreeCollapseElement be boolean? I saw these defaulted to false in the default options

--Missing Documentation
toggleSelectRow
setMaxPage
getGroupedData
columnHeaderSortMulti

--Source Code

  • Tabulator.prototype.getGroups - values parameter is not used
  • Tabulator.prototype.getGroupedData - should that function not accept an active param to pass to getData?

-It appears you are already doing the work required to get the full column definition when calling getColumnLayout. If you really wanted to persist column definition to the database you would need the full definition. Consider returning full ColumnDefinition instead of a subset of ColumnDefinition.
There is a getColumnDefinition but not a setColumnDefinition. This seems like it could be improved.

-The set of methods updateOrAddRow,updateRow, etc are similar to updateOrAddData,updateData, etc. The only difference seems whether updating a single row objects or multiple. Why not consolidate this into a single method or rename the data type functions to "rows". i.e updateOrAddRows. Furthermore,the use of the word "data" is most associated with ajax type methods.

@olifolkerd
Copy link
Owner

olifolkerd commented Mar 8, 2019

Hey @Jojoshua

Thanks for your hard work in putting this together!

As im not a TypeScript user myself, would you mind taking me through the difference of storing the Typings in the Tabulator repo vs putting them in DefinitelyTyped?

The Tabulator API changes a little bit each release, what would be the update process for the typings?

Also is there anything i should add to the documentation to explain that Tabulator has Typing and an example of how to use them?

Cheers

Oli :)

@angeliski
Copy link
Contributor

I believe is better and more simple put the typings together with Tabulator just to faster release and installation.
But I don't know advantages in use DefinitelyTyped...

Great Job @Jojoshua !

@olifolkerd
Copy link
Owner

@angeliski I would feel a bit more comfortable that way to as it means i have a bit of control over updating then when i do a new version release, so there is less risk of things becoming out of sync.

But i am willing to hear other points of views.

Cheers

Oli :)

@Jojoshua
Copy link
Contributor

Jojoshua commented Mar 8, 2019

@olifolkerd I've taken the guidance from the typescript docs here https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

Basically it is the defacto central location that all typescript users can go to find typings; there are thousands of libraries there. If the project itself was written in typescript it can ship the index.d.ts and there is a way in the tsconfig to automatically pick it up that way.

It is typical that the APIs for libraries change and the method that they have at DefinitelyTyped handles that well. Basically you can create a pull request for the definition file I made and push an update to it yourself. Anyone listed in the headers section (http://definitelytyped.org/guides/contributing.html) will be notified and can also review the update. There can be multiple authors noted in the header with version as well.

I also think if you add yourself as an author that their system will recognize your github user as the Author is the Owner of the package and will be merged faster. Check out some more of the details http://definitelytyped.org/ and https://github.com/DefinitelyTyped/DefinitelyTyped

@andreivanea
Copy link
Author

andreivanea commented Mar 8, 2019

@olifolkerd DefinitelyTyped is where I first looked for the types before coming and asking here. It's basically where it all starts (simple and naive exageration, of course...). Another advantage of DefinitelyType is that you can use npm to add the types to your project.

@Jojoshua
Copy link
Contributor

Jojoshua commented Mar 8, 2019

Yep. Speaking of which it just got merged and published. Here it is to test out. https://www.npmjs.com/package/@types/tabulator-tables

@Jojoshua
Copy link
Contributor

Jojoshua commented Mar 8, 2019

@olifolkerd I didn't see your last question. You can mention that there is typescript support but it's not necessarily to explain how to use it. You can add a badge if you want http://definitelytyped.org/pages/badges.html

Anyone doing typescript knows to just pull the definitions. Check the test file I made along side the definition file. I basically took your code from the website to validate it works. I didn't test everything (there is so much content) so there may be some issues to work out and adjust definitions.

You should try a little typescript and test it out, I think you would like it.

@angeliski
Copy link
Contributor

Hey @Jojoshua I have tried use that package, but didn't success.
Do you have a repository example where you do that? Maybe I have miss something, I not a typescript pro

@olifolkerd
Copy link
Owner

Hey @Jojoshua

Sorry for the delay in getting back to you on this one, Ive had a crazy busy couple of months.

Im preparing the 4.3 release so would love to put togeather some docs on how to use the typings.

Would you be able to put togeather a few paragraphs i could use on a TypeScript Typings doc page?

Cheers

Oli :)

@Jojoshua
Copy link
Contributor

Jojoshua commented May 9, 2019

Hey @angeliski what was the issue you were having?

@Jojoshua
Copy link
Contributor

Jojoshua commented May 9, 2019

Hey @Jojoshua

Sorry for the delay in getting back to you on this one, Ive had a crazy busy couple of months.

Im preparing the 4.3 release so would love to put togeather some docs on how to use the typings.

Would you be able to put togeather a few paragraphs i could use on a TypeScript Typings doc page?

Cheers

Oli :)

Hey @olifolkerd , for TypeScript users they should download the definition from @types/tabulator-tables and that's pretty much it.

I have created a sample repository for reference here https://github.com/Jojoshua/TypedTabulator . You should be able to test the bounds of the definitions using this sample project.

@angeliski
Copy link
Contributor

Hey @Jojoshua
I create a PR with my problem: https://github.com/angeliski/vue-tabulator/pull/2/files
I just removed my module declaration and install the definition, but I get the error:
typescript
As a suggestion to documentation, it will be pretty nice if the types are inside of each page.
Thank you

@Jojoshua
Copy link
Contributor

Jojoshua commented May 9, 2019

@angeliski I don't have experience with vue. Why are you explicitly declaring it as a module? Have you tried removing that module declaration altogether?

@angeliski
Copy link
Contributor

No problem @Jojoshua
I setup another project, just to reflect the problem: https://github.com/angeliski/TypeScript-Babel-Starter
The module declaration is removed, you can see the // on start the line.

@Jojoshua
Copy link
Contributor

Jojoshua commented May 9, 2019

@angeliski i can't look now but check out my repo. It doesn't look like you have @types setup in your tsconfig

@Jojoshua
Copy link
Contributor

Jojoshua commented May 9, 2019

"typeRoots": ["node_modules/@types"]

@angeliski
Copy link
Contributor

@Jojoshua I put the typeRoots in the tsconfig, but the error persists

@angeliski
Copy link
Contributor

Hey @Jojoshua I found the problem. Your package is correct, the problem is in the ES6 style imports.
You can see here a long explanation.
Maybe we can put that in the docs, for reference.

Thank you for the support

@Jojoshua
Copy link
Contributor

Jojoshua commented May 9, 2019

@angeliski I'm glad you figured it out. I am curious about the explanation but the link you provided didn't take me to a specific explanation. Can you try again?

@angeliski
Copy link
Contributor

@Jojoshua I read again and you are right. The problem is not that yet. The link is more a workaround for that situation. I will try search more to find a really solution.

@Jojoshua
Copy link
Contributor

Thanks @angeliski so what did you change to stop the error?

@angeliski
Copy link
Contributor

In the TypeScript-Babel-Starter project I just change import to require, how you can see there, but in my Vue project does not work. :/

@Jojoshua
Copy link
Contributor

@angeliski if you try to use the :Tabulator type inside your main.ts file it should work because that is a typescript file. I don't know how it works in .vue files.

@angeliski
Copy link
Contributor

Solved @Jojoshua .
The problem is my tsconfig, I find a explanation here. I put the tabulator-tables inside the types array and work.
If can view my PR here.
Thank you for the support and for the types in Tabulator!

@Jojoshua
Copy link
Contributor

Thanks @angeliski for letting us know the solution!

@olifolkerd
Copy link
Owner

Hey @Jojoshua

Thanks for the info, i will be releasing an update to the website this weekend so will include a link to the typings then.

Cheers

Oli :)

@olifolkerd
Copy link
Owner

Hey All,

I have added a new "Languages" section to the documentation to cover support for Tabulator in other languages, it will include some typescript documentation along with other info

it should be going live later today

Cheers

Oli :)

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

No branches or pull requests

4 participants