-
Notifications
You must be signed in to change notification settings - Fork 36
feat: support configuring custom endpoints in loader scripts #28
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
Conversation
| } from '../runner/reporting/report-local-disk'; | ||
|
|
||
| const app = express(); | ||
| const reportsLoaderPromise = getReportLoader(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why I had it set up this way with a lazy promise was that the CLI's dev server was a bit flaky if the the boot-up sequence for the Express server is asynchronous. I think we may have to keep that so it shows up reliably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean with flaky?
I saw some flakiness/or delay of the server when booting up, but that was because we printed the "Server is ready at: localhost:XX" too early. I've fixed this by leveraging the Express "listen" callback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was happening either if I had the server already running and I restarted it, or if I go to localhost:4200 too quickly. The CLI would throw some error since the server hadn't booted up yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen anything like this (after adding the listen Express hook). How do we proceed?
We need to await so that we can hook up endpoints.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's proceed as is and we can think of something else if it shows up again.
| } from '../runner/reporting/report-local-disk'; | ||
|
|
||
| const app = express(); | ||
| const reportsLoaderPromise = getReportLoader(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's proceed as is and we can think of something else if it shows up again.
This is useful for advanced users who might upload images to e.g. Firebase Storage and then want to proxy image URLs to augment with an `Authorization` header.
2ac84a6 to
c47d6e2
Compare
This is useful for advanced users who might upload images to e.g. Firebase Storage and then want to proxy image URLs to augment with an
Authorizationheader.