feat(dev-server): add "ping" route #5414
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds a new `/ping` route to the dev server handler. This route will return a 200 response once the Stencil build has finished.
This commit adds an option to the dev server config object as a part of the Stencil config to allow users to change the route for the "ping" response. The user supplied route is validated when validating the dev server config. If the user sets the ping route to `null`, the route will not be registered.
tanner-reits
commented
Feb 26, 2024
| * | ||
| * Defaults to `/ping` | ||
| */ | ||
| pingRoute?: string | null; |
There was a problem hiding this comment.
Went with pingRoute, but totally fine if we wanted to go with something like "health" or "status".
|
| Path | Location | Error | Message |
|---|---|---|---|
| src/compiler/config/test/validate-dev-server.spec.ts | (284, 14) | TS18048 | |
| src/dev-server/request-handler.ts | (44, 25) | TS2345 | |
| src/dev-server/request-handler.ts | (48, 25) | TS2345 | |
| src/dev-server/request-handler.ts | (52, 33) | TS2345 | |
| src/dev-server/request-handler.ts | (70, 46) | TS2345 | |
| src/dev-server/request-handler.ts | (85, 45) | TS2345 | |
| src/dev-server/request-handler.ts | (85, 67) | TS18048 | |
| src/dev-server/request-handler.ts | (130, 5) | TS2322 | |
| src/dev-server/request-handler.ts | (131, 5) | TS2322 | |
| src/dev-server/request-handler.ts | (132, 5) | TS2322 | |
| src/dev-server/request-handler.ts | (138, 25) | TS2345 | |
| src/dev-server/request-handler.ts | (140, 25) | TS2345 | |
| src/dev-server/request-handler.ts | (150, 51) | TS18048 | |
| src/dev-server/request-handler.ts | (153, 59) | TS2345 | |
| src/dev-server/request-handler.ts | (169, 61) | TS18048 |
reports and statistics
Our most error-prone files
| Path | Error Count |
|---|---|
| src/dev-server/index.ts | 37 |
| src/dev-server/server-process.ts | 32 |
| src/compiler/prerender/prerender-main.ts | 22 |
| src/testing/puppeteer/puppeteer-element.ts | 22 |
| src/runtime/client-hydrate.ts | 20 |
| src/screenshot/connector-base.ts | 19 |
| src/runtime/vdom/vdom-render.ts | 17 |
| src/dev-server/request-handler.ts | 15 |
| src/compiler/prerender/prerender-optimize.ts | 14 |
| src/compiler/sys/stencil-sys.ts | 14 |
| src/compiler/transpile/transpile-module.ts | 14 |
| src/sys/node/node-sys.ts | 14 |
| src/compiler/prerender/prerender-queue.ts | 13 |
| src/compiler/sys/in-memory-fs.ts | 13 |
| src/runtime/connected-callback.ts | 13 |
| src/runtime/set-value.ts | 13 |
| src/compiler/output-targets/output-www.ts | 12 |
| src/compiler/transformers/test/parse-vdom.spec.ts | 12 |
| src/compiler/transformers/transform-utils.ts | 12 |
| src/mock-doc/test/attribute.spec.ts | 12 |
Our most common errors
| Typescript Error Code | Count |
|---|---|
| TS2322 | 361 |
| TS2345 | 349 |
| TS18048 | 202 |
| TS18047 | 82 |
| TS2722 | 37 |
| TS2532 | 24 |
| TS2531 | 21 |
| TS2454 | 14 |
| TS2790 | 11 |
| TS2352 | 10 |
| TS2769 | 8 |
| TS2538 | 8 |
| TS2416 | 6 |
| TS2493 | 3 |
| TS18046 | 2 |
| TS2684 | 1 |
| TS2430 | 1 |
Unused exports report
There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!
Unused exports
| File | Line | Identifier |
|---|---|---|
| src/runtime/bootstrap-lazy.ts | 21 | setNonce |
| src/screenshot/screenshot-fs.ts | 18 | readScreenshotData |
| src/testing/testing-utils.ts | 198 | withSilentWarn |
| src/utils/index.ts | 145 | CUSTOM |
| src/utils/index.ts | 269 | normalize |
| src/utils/index.ts | 7 | escapeRegExpSpecialCharacters |
| src/compiler/app-core/app-data.ts | 25 | BUILD |
| src/compiler/app-core/app-data.ts | 115 | Env |
| src/compiler/app-core/app-data.ts | 117 | NAMESPACE |
| src/compiler/fs-watch/fs-watch-rebuild.ts | 123 | updateCacheFromRebuild |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | satisfies |
| src/compiler/types/validate-primary-package-output-target.ts | 61 | Record |
| src/testing/puppeteer/puppeteer-declarations.ts | 485 | WaitForEventOptions |
| src/compiler/sys/fetch/write-fetch-success.ts | 7 | writeFetchSuccessSync |
PR built and packed!Download the tarball here: https://github.com/ionic-team/stencil/actions/runs/8068438714/artifacts/1279517748 If your browser saves files to |
rwaskiewicz
suggested changes
Feb 26, 2024
| * | ||
| * Defaults to `/ping` | ||
| */ | ||
| pingRoute?: string | null; |
christian-bromann
approved these changes
Feb 26, 2024
Co-authored-by: Ryan Waskiewicz <[email protected]>
tanner-reits
force-pushed
the
treits/feat/dev-server-ping-route
branch
from
f9e2df0
to
ad2cefd
Compare
rwaskiewicz
approved these changes
Feb 27, 2024
|
This has been released with Stencil 🚞 v4.13.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the current behavior?
The Stencil dev server will "spin up" prior to the accompanying project build completing. This can cause issues for processes (such as E2E testing tools like Playwright), that rely on the dev server to work correctly.
GitHub Issue Number: N/A
What is the new behavior?
A new route is added to the dev server request handler. This route will return a 200 response once the Stencil build is completed and successful. If a build fails, or another error is encountered, the route will return a 500 error.
This route is configurable via the
devServerobject on the Stencil config. If the route is explicitly set tonull, no route will be registered with the handler.Documentation
ionic-team/stencil-site#1363
Does this introduce a breaking change?
Testing
Automated tests
Unit tests were added to the request handler and validation test suites.
Manual tests
Functionality can be tested by installing a build of this branch into any Stencil project. After running
npm start, navigate to/ping. Can also test changing the value in the project's config or setting itnull.Other information
This change was made as a part of work to implement a Playwright testing adapter for Stencil. Prior to this change, tests would start execution prematurely, leading to quick failures.