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

chore(typescript): Migrate @feathersjs/errors to modern classes and TypeScript #1920

Merged
merged 5 commits into from
Apr 16, 2020

Conversation

daffl
Copy link
Member

@daffl daffl commented Apr 16, 2020

This pull request uses modern ES classes for error types and removes undocumented initialization modes and workarounds for browser engines that are no longer supported.

@daffl daffl added this to the v5 (Dove) milestone Apr 16, 2020
daffl added a commit to feathersjs-ecosystem/docs that referenced this pull request Apr 16, 2020
@daffl daffl merged commit 17d80e0 into dove Apr 16, 2020
@daffl daffl deleted the ts-errors branch April 16, 2020 21:38
@@ -3,8 +3,8 @@
"description": "Common error types for Feathers apps",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this package use tabs and spaces?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was wondering why it looked odd. Fixed in cbd31c1

@bertho-zero
Copy link
Collaborator

Cool, I think we should integrate the concept of verror in this package, it's really practical.

The problem with verror is that it uses heavy dependencies and is not compatible for browsers, but it is easily replaceable.

https://github.com/joyent/node-verror

@daffl
Copy link
Member Author

daffl commented Apr 17, 2020

Interesting, I hadn't seen that before. Hoping that this refactoring as a first step combined with the new hooks will also improve error messages (and stack traces).

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

Successfully merging this pull request may close these issues.

2 participants