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

[Console] Convert all lib/mappings files to typescript #3800

Closed
zhongnansu opened this issue Apr 6, 2023 · 8 comments · Fixed by #4008
Closed

[Console] Convert all lib/mappings files to typescript #3800

zhongnansu opened this issue Apr 6, 2023 · 8 comments · Fixed by #4008
Assignees
Labels
console good first issue Good for newcomers technical debt If not paid, jeapardizes long-term success and maintainability of the repository. typescript

Comments

@zhongnansu
Copy link
Member

zhongnansu commented Apr 6, 2023

Coming from #3775 (comment), move to ts will make code more readable and maintainable.

Files:

find ./src/plugins/console/public/lib/mappings -name "*.js"
./src/plugins/console/public/lib/mappings/__tests__/mapping.test.js
./src/plugins/console/public/lib/mappings/mappings.js
@zhongnansu zhongnansu added technical debt If not paid, jeapardizes long-term success and maintainability of the repository. console labels Apr 6, 2023
@joshuarrrr joshuarrrr changed the title [Console] Migrate mapping.js to typescript [Console] Convert all lib/mappings files to typescript Apr 11, 2023
@joshuarrrr joshuarrrr added typescript good first issue Good for newcomers and removed untriaged labels Apr 11, 2023
@joshuarrrr joshuarrrr removed their assignment Apr 11, 2023
@Seitakhmetv
Copy link

✌️Hello!
Based on the issue, to increase readability and maintainability in those files, I'd like to suggest following

Suggestions:

  • Add typings for the parameters, variables, and return values
  • Update function signatures with TS types based on the typings above
  • Replace typeof and Array.isArray with TS type guards
  • Use TS utility types, such as Record and Partial
  • Update the entire "lodash" import to individual functions import
  • Add TS interfaces to provide type safety

Could you, please, assign this issue to me?

Feel free to comment or add more suggestions

@zhongnansu
Copy link
Member Author

@Seitakhmetv The suggestions look great! Let me assign this to you, looking forward to your pull requests!

@Seitakhmetv
Copy link

@Seitakhmetv In case you hadn't found these docs

Thank you!
Those are really useful for me and, yes, indeed I haven't seen them.
Today I'll finish typescripting files above.

@curq
Copy link
Collaborator

curq commented May 7, 2023

Hi @Seitakhmetv, I hope you're doing well. I was wondering about the progress on this issue you've been working on. I'm looking forward to using the types from the mapping, as without them I can't complete my related Typescript conversion issue. Could you please provide an update on the status or an estimated time for completion? Thank you!

@curq
Copy link
Collaborator

curq commented May 11, 2023

Hello, @Seitakhmetv @joshuarrrr Can I make a PR for this issue? My two other issues need types from this one to finish, so I'm currently blocked because of it.

@joshuarrrr
Copy link
Member

@curq Yes, go ahead if you're ready to unblock the related work.

@curq
Copy link
Collaborator

curq commented May 11, 2023

@joshuarrrr I made a PR, could you check it please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console good first issue Good for newcomers technical debt If not paid, jeapardizes long-term success and maintainability of the repository. typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants