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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(example-greeting-app): add an example greeting app on top of greeter extension #2941

Merged
merged 2 commits into from May 30, 2019

Conversation

Projects
None yet
6 participants
@raymondfeng
Copy link
Member

commented May 22, 2019

Introducing a new greeting-app example on top of greeter-extension to prepare for a series of tutorials #2939:

  1. Add REST API for greeting service
  2. Show case caching with interceptor and life cycle observer
  3. Use @config.* and ctx.configure

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • 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

馃憠 Check out how to submit a PR 馃憟

@raymondfeng raymondfeng requested a review from bajtos as a code owner May 22, 2019

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch from 5541c1e to 52d20fa May 23, 2019

@raymondfeng raymondfeng changed the title feat(example-greeter-extension): add REST api to expose greeting service feat(example-greeter-extension): add more features for greeting app May 23, 2019

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch 3 times, most recently from 13bc751 to 603ed8d May 23, 2019

@bajtos
Copy link
Member

left a comment

I am confused. Initially, example-greeter-extension was created to show how to create an extension (a LB4 Component). Now it looks more like a regular application.

How do you envision 3rd party applications to use example-greeter-extension as a Component?

If example-greeter-extension is no longer a Component, then we should rename it accordingly, e.g. example-greeter.

}

main();

This comment has been minimized.

Copy link
@bajtos

bajtos May 24, 2019

Member

I find it suspicious that main is always executed.

I believe we are using the following approach in other places:

if (require.main === module) {
  main().catch(err => {
    console.error(err);
    process.exit(1);
  });
}

async function main() {
  const app = new module.exports.GreetingApplication({
    // ...
  }
}
@raymondfeng

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

I am confused. Initially, example-greeter-extension was created to show how to create an extension (a LB4 Component). Now it looks more like a regular application.

I'm trying to reuse this scenario for #2939.

How do you envision 3rd party applications to use example-greeter-extension as a Component?

The module can still be imported by another app, such as:

import {GreetingComponent} from '@loopback/example-greeter-extension';

app.component(GreetingComponent);

Maybe it makes sense to break the features into two packages:

  1. example-greeter-extension - stay as a component
  2. example-greeter-app - a new app that uses example-greeter-extension and adds new features such as REST APIs and caching

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch from 603ed8d to a85e5a1 May 24, 2019

@raymondfeng

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

FYI: I went ahead to refactor the code as follows:

  1. example-greeter-extension - stay as a component
  2. example-greeter-app - a new app that uses example-greeter-extension and adds new features such as REST APIs and caching

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch from c394e3c to 07730aa May 24, 2019

@raymondfeng raymondfeng changed the title feat(example-greeter-extension): add more features for greeting app feat(example-greeting-app): add an example greeting app on top of greeter extension May 24, 2019

@raymondfeng raymondfeng requested a review from bajtos May 24, 2019

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch 2 times, most recently from a8e0739 to 9899163 May 24, 2019

@dhmlau

dhmlau approved these changes May 29, 2019

Copy link
Contributor

left a comment

I ran the app and reviewed the content, LGTM. But I don't have too much knowledge on this. :)

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch from 9899163 to 8b13ac3 May 29, 2019

@b-admike
Copy link
Member

left a comment

I would really like to see a bit more beefy README file for the new greeter app. Can we explain a bit more on the caching interceptor/observer/service? Also, we should let our users know that they need to set Accept-Language header when trying it out and modify the path /greeter/<Name> to play with it. It wouldn't hurt to mention to turn on debug strings with DEBUG=greeter-extension* e.g. to showcase what the caching observer/interceptor are doing in the background. It'd be nice to include explorer with the app if it can allow users to put in the headers for the endpoint (I know we're trying to keep this lightweight).

Show resolved Hide resolved examples/greeting-app/README.md Outdated
Show resolved Hide resolved examples/greeting-app/README.md Outdated
Show resolved Hide resolved examples/greeting-app/README.md
Show resolved Hide resolved examples/greeting-app/src/caching-service.ts

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch 2 times, most recently from 79a64a0 to 7c2cff6 May 29, 2019

@raymondfeng

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

I would really like to see a bit more beefy README file for the new greeter app. Can we explain a bit more on the caching interceptor/observer/service? Also, we should let our users know that they need to set Accept-Language header when trying it out and modify the path /greeter/ to play with it. It wouldn't hurt to mention to turn on debug strings with DEBUG=greeter-extension* e.g. to showcase what the caching observer/interceptor are doing in the background. It'd be nice to include explorer with the app if it can allow users to put in the headers for the endpoint (I know we're trying to keep this lightweight).

I'm trying to cover them in more details at #2939.

@raymondfeng

This comment has been minimized.

Copy link
Member Author

commented May 29, 2019

@b-admike Thank you for the feedback. PTAL.

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch 2 times, most recently from 14c477b to 1f461b0 May 29, 2019

@dhmlau dhmlau added this to the June 2019 milestone milestone May 30, 2019

@dhmlau dhmlau added the p1 label May 30, 2019

@raymondfeng raymondfeng requested a review from b-admike May 30, 2019

@b-admike
Copy link
Member

left a comment

LGTM 馃憦. This is an awesome example and I look forward for the tutorials utilizing it as well.

@emonddr
Copy link
Contributor

left a comment

Very cool!

Show resolved Hide resolved examples/greeting-app/README.md Outdated
Show resolved Hide resolved examples/greeting-app/README.md Outdated
Show resolved Hide resolved examples/greeting-app/README.md Outdated
Show resolved Hide resolved examples/greeting-app/index.d.ts
Show resolved Hide resolved examples/greeting-app/index.js
Show resolved Hide resolved examples/greeting-app/src/caching-service.ts

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch from 1f461b0 to c4a612a May 30, 2019

@raymondfeng raymondfeng force-pushed the add-greeting-controller branch from c4a612a to f0f3ca3 May 30, 2019

@raymondfeng raymondfeng merged commit e13bcc7 into master May 30, 2019

4 checks passed

clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 91.916%
Details

@raymondfeng raymondfeng deleted the add-greeting-controller branch May 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.