Skip to content

Remove util.inherits #70

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

Closed
jwerre opened this issue Nov 15, 2021 · 7 comments · Fixed by #109
Closed

Remove util.inherits #70

jwerre opened this issue Nov 15, 2021 · 7 comments · Fixed by #109
Labels
code quality 🧽 Relating to code quality good first issue ✅ Good for newcomers
Milestone

Comments

@jwerre
Copy link
Contributor

jwerre commented Nov 15, 2021

Use ES6 Class syntax with extends instead of utils.inherits

util.inherits(AuthorizationCodeGrantType, AbstractGrantType);

util.inherits(ClientCredentialsGrantType, AbstractGrantType);

util.inherits(PasswordGrantType, AbstractGrantType);

util.inherits(RefreshTokenGrantType, AbstractGrantType);

This also occurs in all the errors at lib/errors

@HappyZombies
Copy link
Member

Wouldn't this require to change these functions to be class AuthorizationCodeGrantType extends AbstractGrantType?

@jwerre
Copy link
Contributor Author

jwerre commented Nov 15, 2021

Yes.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Nov 15, 2021

Try out https://lebab.unibtc.me/editor

@jankapunkt jankapunkt added the code quality 🧽 Relating to code quality label Nov 16, 2021
@jankapunkt
Copy link
Member

Since #58 is merged I could start working on this on a new branch

@jankapunkt jankapunkt added the good first issue ✅ Good for newcomers label Nov 17, 2021
@dsschiramm
Copy link

I'll start with this issue ok?
Seems like a good start for me (first time in a project doing something in an opensource project)

@dsschiramm
Copy link

Should I change the way the import is written as well?

Example:
const AbstractGrantType = require('./abstract-grant-type');
To
import AbstractGrantType from './abstract-grant-type';

Or should I leave this change for another issue/pull request?

@Uzlopak
Copy link
Collaborator

Uzlopak commented Dec 18, 2021

Dont change, as it would just open the whole can of worms of esModule imports potentially breaking while running on node because of some reasons.

dsschiramm pushed a commit to dsschiramm/node-oauth2-server that referenced this issue Dec 18, 2021
dsschiramm pushed a commit to dsschiramm/node-oauth2-server that referenced this issue Dec 19, 2021
dsschiramm pushed a commit to dsschiramm/node-oauth2-server that referenced this issue Dec 19, 2021
@jankapunkt jankapunkt linked a pull request Dec 19, 2021 that will close this issue
@jankapunkt jankapunkt added this to the v5 milestone Aug 25, 2022
jankapunkt added a commit that referenced this issue Jun 8, 2023
 Merge pull request #109 from dsschiramm/development
Thanks to @dsschiramm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality 🧽 Relating to code quality good first issue ✅ Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants