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

port to ts #654

Closed
wants to merge 16 commits into from
Closed

port to ts #654

wants to merge 16 commits into from

Conversation

fratzinger
Copy link
Collaborator

@fratzinger fratzinger commented Feb 19, 2022

Move the code base to typescript. Just a refactor, nothing should break but imports from require(feathers-hooks-common/lib/...) or require(feathers-hooks-common/types)

We should make it a major version, just to be sure.

What do you think @daffl ?

Issues: closes #656
PRs: closes #662, #646, #634

@fratzinger fratzinger changed the title WIP: port to ts port to ts May 10, 2022
@daffl
Copy link
Member

daffl commented May 12, 2022

Amazing! I wonder how many things for this would break/change with the latest v5. It does have a good chunk of TypeScript related changes - especially around the hook context.

- `existsByDot` and `data[name]` do basically the same. So `data[name]` is unnecessary
@fratzinger
Copy link
Collaborator Author

I don't know yet. I'm definitely up to work on that. But I think we could merge this first.

What do you think @daffl? Also with #623 . What's the best strategy?

Possible strategies:

  1. Move feathers-hooks-common to ts for feathers v4, port Dove Support #623 to ts and add move feathers v5 stuff
  2. Leave feathers-hooks-common for feathers v4 as is and work on a ts version for feathers v5

However the strategy will be, I would love to work on open PR to clean up the repo:

@daffl
Copy link
Member

daffl commented May 17, 2022

We can totally merge this one first once you put it into the ready for review state.

@fratzinger fratzinger marked this pull request as ready for review May 17, 2022 15:26
@fratzinger
Copy link
Collaborator Author

It's ready 😊

@fratzinger fratzinger mentioned this pull request May 21, 2022
@fratzinger
Copy link
Collaborator Author

continuing on #663

@fratzinger fratzinger closed this May 21, 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.

IDE (VSCode) confused?
3 participants