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

Fix on typings by duplicating the most general listener #106

Merged
merged 1 commit into from
Nov 29, 2018

Conversation

thinkh
Copy link
Member

@thinkh thinkh commented Nov 26, 2018

Related to datavisyn/tdp_core#138 (comment)

prerequisites:

  • branch is up-to-date with the branch to be merged with, i.e. develop
  • build is successful
  • code is cleaned up and formatted
  • tested with Firefox 52, Firefox 57+, Chrome 64+, MS Edge 16+

Summary

As described in datavisyn/tdp_core#138 (comment) some columns are missing the general on listener in the generated *.d.ts file.

I fixed this by adding the line on(type: string | string[], listener: IEventListener | null): this; to every file.

@sgratzl
Copy link
Member

sgratzl commented Nov 26, 2018

why does it work in the released / master branch with the develop branch of tdp_core without these changes?

see https://github.com/datavisyn/lineupjs/blob/master/src/model/NumberColumn.ts#L166

@thinkh
Copy link
Member Author

thinkh commented Nov 26, 2018

@sgratzl I don't know why it works with the previous version, but not with the current version. I set up a fresh Ordino (public) and switch the branches. The suggested changes are the only solution could find to fix the reported errors.

@thinkh
Copy link
Member Author

thinkh commented Nov 28, 2018

I tried this again, but I couldn't figure out a solution.

Steps to reproduce

  1. yo phovea:setup-workspace ordino_product -b open_source
  2. npm run start -> compiles works without errors
  3. npm install lineupjs@4.0.0-alpha.8
  4. npm run start -> numerous compilation errors, also the one's described in Update LineUp to v4.0.0-alpha.9 datavisyn/tdp_core#138 (comment)

@sgratzl If you think this suggested PR is not a proper solution, it would be good, if you can have a look what might cause the problem and how to solve it. Thanks.

@sgratzl sgratzl merged commit b9500fc into develop Nov 29, 2018
@sgratzl sgratzl deleted the thinkh/fix-on-listener-typings branch November 29, 2018 20:54
@thinkh
Copy link
Member Author

thinkh commented Nov 30, 2018

Thanks for merging.

thinkh added a commit that referenced this pull request Sep 16, 2019
@thinkh thinkh mentioned this pull request Sep 16, 2019
4 tasks
@thinkh thinkh mentioned this pull request Oct 3, 2019
28 tasks
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.

None yet

2 participants