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

refactor(authentication): simplify decorators import path #1991

Merged
merged 3 commits into from Nov 7, 2018

Conversation

Yaty
Copy link
Contributor

@Yaty Yaty commented Nov 7, 2018

Use decorators index when importing

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@Yaty
Copy link
Contributor Author

Yaty commented Nov 7, 2018

Hmm, I do not understand why prettier is failing on Travis. The error is really not clear.
It's passing on my machine and my branch is updated.
Any ideas ?

@raymondfeng
Copy link
Contributor

@Yaty It seems that Travis build picks up a newer version of prettier (prettier@1.15.1), which causes a few files to be reformatted.

Please upgrade to prettier@1.15.1 in packages/build/package.json and run npm i && npm run lint:fix again to add the formatting changes.

@hacksparrow
Copy link
Contributor

I am opening a PR to update prettier.

@hacksparrow
Copy link
Contributor

Here is the PR - #1993. Let's land this ASAP.

@@ -5,7 +5,7 @@

import {Strategy} from 'passport';
import {AuthenticateFn, UserProfile} from './types';
import {AuthenticationMetadata} from './decorators/authenticate.decorator';
import {AuthenticationMetadata} from './decorators';
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note importing from the target file is not bad. Sometimes it's even necessary to avoid circular dependencies. If a module does not have side-effect code at top level, either way is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely, I'll keep your comment in my mind :)
Here this is just syntactic sugar to ease import of a "future" new decorator.

@raymondfeng raymondfeng merged commit e431178 into loopbackio:master Nov 7, 2018
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

3 participants