Repository navigation
perf(seo): optimize crawlability and loading speed - #118
maximgorbatyuk merged 4 commits into
Conversation
Implement Phase 1 SEO and performance improvements for techinterview.space and /salaries.\n\n- Replace non-crawlable login anchor with semantic button control\n- Add sitemap redirect endpoint and enable compression in SSR server\n- Remove eager lazy-module imports and global Prism bundle; lazy load Prism where needed\n- Add responsive image variants, intrinsic sizing, and loading-priority hints\n- Add salaries loading shell and delayed GIF strategy to reduce CLS\n- Keep robots strategy: /salaries blocked, /salaries/overview allowed\n\nIncludes unit tests for navbar semantics, loading spinner behavior, and sitemap redirect handling.
WalkthroughMoves PrismJS from build-time assets to a runtime PrismLoaderService with lazy imports and highlights; updates components to call the loader; removes Prism assets from angular build; adds compression middleware and a sitemap redirect endpoint; other UI and asset tweaks (navbar button, spinner delay, image attrs, meta tag handling). Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/modules/salaries/components/salaries-chart/salaries-chart.component.ts (1)
86-94:⚠️ Potential issue | 🟡 MinorDead expression on line 91 — result is discarded.
this.filterData.dateTo != null;on line 91 is a standalone boolean expression whose result is unused. It appears it was intended to be part of the compound assignment on lines 87–90 (perhaps|| this.filterData.dateTo != null), but instead it's a no-op statement. This is pre-existing, but worth fixing while you're in this area.Possible fix
this.hasPredefinedFilter = this.filterData.grade != null || this.filterData.profsInclude?.length > 0 || - this.filterData.cities?.length > 0; - this.filterData.dateTo != null; + this.filterData.cities?.length > 0 || + this.filterData.dateTo != null;
🤖 Fix all issues with AI agents
In `@angular.json`:
- Around line 31-37: Prism theme CSS was removed so tokenized code blocks lack
semantic colors; restore loading a Prism theme by either importing a theme CSS
from Prism (e.g., add an import like "prismjs/themes/prism-okaidia.css" inside
PrismLoaderService), or add the theme file to global styles (e.g., include it in
styles.scss or re-add to angular.json "styles"), or implement token-level rules
in _warm-theme.scss to style Prism token classes; update PrismLoaderService,
styles.scss, or angular.json accordingly so the Prism token classes (used by
PrismLoaderService) receive a proper theme.
In `@src/app/components/navbar/navbar.component.spec.ts`:
- Around line 38-48: The test currently uses an if (loginButton) guard which
allows it to pass when no button is rendered; update the "renders login as
button for crawl-safe markup" spec to assert the button exists unconditionally:
remove the conditional and add an explicit expect(loginButton).not.toBeNull()
(or toBeTruthy()) immediately after asserting loginAnchor is null, then continue
to assert loginButton.tagName === "BUTTON" and loginButton.getAttribute("href")
is null; this ensures loginButton is required by the test and will fail if the
component stops rendering it.
In `@src/app/modules/home/components/home/home.component.html`:
- Around line 46-59: The first feature image element (img with class "img-fluid
feature-image" and alt "Графики и статистика зарплат в IT") is marked
loading="lazy" but may appear in the initial viewport on large/tall screens;
change its loading attribute to "eager" (or remove lazy) so the browser loads it
immediately to avoid LCP issues and ensure Lighthouse doesn't flag it.
In
`@src/app/modules/interviews/components/text-with-linebreaks/text-with-linebreaks.component.ts`:
- Around line 43-45: The call inside queueMicrotask that currently does "void
this.prismLoader.loadAndHighlight(this.elementRef.nativeElement)" can yield an
unhandled rejection if loadAndHighlight fails; update the queueMicrotask
callback (in the TextWithLinebreaksComponent) to call
this.prismLoader.loadAndHighlight(this.elementRef.nativeElement).catch(err => {
/* log or handle */ }) so the promise rejection is caught — e.g., use
console.error or the component's logger to record the error and avoid an
unhandled rejection while preserving the highlight attempt.
In `@src/app/shared/components/loading-spinner/loading-spinner.component.html`:
- Around line 10-18: The template's alt text is hardcoded to "Loading dog GIF"
while the component toggles loadingUrl between LoadingCatUrl and LoadingDogUrl;
add a companion property (e.g., loadingAlt) in the LoadingSpinnerComponent and
set it wherever loadingUrl is chosen (use "Loading cat GIF" or "Loading dog GIF"
or a generic "Loading animation"), then update the template to bind
[alt]="loadingAlt" instead of the fixed string; ensure the property is
initialized the same place as loadingUrl and used by onGifLoaded()/other logic
if needed.
In `@src/app/shared/components/loading-spinner/loading-spinner.component.spec.ts`:
- Around line 27-37: The test calls component.ngOnInit() a second time while
beforeEach already invoked fixture.detectChanges() (which calls ngOnInit) and
left a pending timer in delayTimerId; fix by either setting component.delayMs
and component.loadingUrl before the first detectChanges (move
fixture.detectChanges() out of beforeEach into each test so inputs are
configured first) or, if you must call ngOnInit() in the test, clear the
existing timer first by calling clearTimeout(component.delayTimerId) or invoking
component.ngOnDestroy() to cancel the prior timer before reinitializing; update
the test to use fixture.detectChanges() after configuring inputs or perform the
clearTimeout/ngOnDestroy step to avoid leaking the original timer.
In `@src/app/shared/services/prism-loader.service.ts`:
- Around line 40-52: The dynamic import inside the self-invoking async may
reject and leave this.loadPromise rejected forever (breaking subsequent
ensureLoaded()); wrap the import sequence in a try/catch inside the async block
used to set loadPromise, and on any error reset this.loadPromise = null (and
keep this.prism = null) then rethrow the error so callers know it failed and
future ensureLoaded() calls can retry; refer to loadPromise, ensureLoaded(),
prism, and prismModule to locate where to add the try/catch and reset logic.
🧹 Nitpick comments (11)
src/app/modules/home/components/home/home.component.html (1)
46-60: Mixed-originsrcset: local assets for smaller variants, CDN for 1000w.The 480w and 800w variants are served from
/assets/img/home/(same origin), while the 1000w fallback comes from the DigitalOcean CDN. This pattern repeats across all four feature images and the hero. A few things to keep in mind:
- If the CDN is unreachable or slow, only desktop users requesting the largest variant are affected — the
srcfallback is local, so degradation is graceful.- Ensure the CDN responds with appropriate
Access-Control-Allow-Originheaders if you ever addcrossoriginto these images (e.g., for canvas/performance-observer use).Not a blocker, but worth noting for operational awareness.
src/robots.txt (2)
1-10: Consider adding explicit Allow rules for consistency.The
User-agent: *section lacks explicitAllow: /companiesandAllow: /rules that are present in the AI crawler sections (lines 16-17, 22-23, etc.). While robots.txt allows everything by default unless disallowed, adding these explicit rules would improve consistency and documentation.📝 Suggested enhancement for consistency
User-agent: * # Private / auth-related routes Disallow: /admin Disallow: /me Disallow: /auth-callback Disallow: /logout-callback # Keep salaries dashboard blocked; public SEO landing is /salaries/overview Allow: /salaries/overview Disallow: /salaries +Allow: /companies +Allow: /
13-41: AI crawler rules duplicate User-agent: * rules.Lines 13-41 explicitly repeat the same Allow/Disallow rules for each AI crawler that are already defined in the
User-agent: *section (lines 1-10). While this redundancy is not harmful and may be intentional for documentation or future-proofing, it creates a maintenance burden—any rule changes must be replicated across six sections (User-agent: * + 5 AI crawlers).If the rules for these AI crawlers are identical to the general rules, consider removing the redundant sections or documenting the rationale for keeping them.
src/app/components/navbar/navbar.component.html (1)
79-88: Good change: anchor → button improves crawlability and semantics.One minor cleanup — the conditional
disabledCSS class on lines 81–83 is redundant now that you use the native[disabled]binding (line 85). Buttons natively support:disabled, so Bootstrap's.disabledclass (designed for<a>elements) is unnecessary here.♻️ Suggested simplification
<button type="button" - class="btn btn-primary login-btn {{ - !loginButtonAvailable ? 'disabled' : '' - }}" + class="btn btn-primary login-btn" (click)="login()" [disabled]="!loginButtonAvailable" > Войти </button>src/app/modules/salaries/components/salaries-chart/salary-block-value/salary-block-value.component.html (1)
41-50: Consider a more descriptivealtattribute.The
alt="logo"is generic. Thetitleattribute already has a meaningful description ("Кот Магеллан, который любит считать деньги"). Consider using a similarly descriptivealtvalue — or, if the image is purely decorative, usealt=""to let screen readers skip it.Also,
loading="eager"is fine if this image is reliably above the fold, but on mobile wherecol-md-3stacks below the content, it may be off-screen initially —loading="lazy"could save bytes on mobile viewports.src/app/modules/salaries/components/salaries-chart/salaries-chart.component.scss (1)
39-42: Consider using a shared SCSS breakpoint variable instead of the hardcoded768px.If the project defines responsive breakpoints as SCSS variables (e.g.,
$breakpoint-md), using them here would improve consistency. Minor nit.src/server.ts (1)
96-99: Updateapp.listen()error handling to use the'error'event.In Express 5 (currently installed as
^5.1.0),app.listen()delegates to Node'shttp.Server.listen(), whose callback is invoked on the'listening'event with no arguments. Theerrorparameter on line 96 will always beundefined. Startup errors (e.g., port in use) surface as an'error'event on the server instead. Capture the return value fromapp.listen()and attach a.on('error', ...)handler:Suggested fix
const server = app.listen(port, () => { console.log(`Node Express server listening on http://localhost:${port}`); }); server.on('error', (error) => { throw error; });src/app/shared/services/prism-loader.service.ts (1)
64-65: Static analysis:forEachcallback implicitly returns a value.The arrow function returns the result of
highlightElement. Use a block body to satisfy the linter and clarify intent.Proposed fix
- codeBlocks.forEach((block) => this.prism!.highlightElement(block)); + codeBlocks.forEach((block) => { + this.prism!.highlightElement(block); + });src/app/modules/interviews/components/text-with-linebreaks/text-with-linebreaks.component.ts (1)
28-34: Double highlighting on initial render.Angular invokes
ngOnChangesbeforengOnInitwhen inputs are bound, soupdateStateAndHighlight()fires twice on the first render — queuing twoloadAndHighlightmicrotasks. The highlighting is idempotent so this isn't a bug, but it's unnecessary work.A common pattern is to drop
OnInitand rely solely onngOnChanges, or guard with aninitializedflag.♻️ Option: remove OnInit and rely on OnChanges alone
-export class TextWithLinebreaksComponent implements OnInit, OnChanges { +export class TextWithLinebreaksComponent implements OnChanges { `@Input`() source: string | null = null; hasText = false; constructor( private readonly elementRef: ElementRef<HTMLElement>, private readonly prismLoader: PrismLoaderService, ) {} - ngOnInit(): void { - this.updateStateAndHighlight(); - } - ngOnChanges(_: SimpleChanges): void { this.updateStateAndHighlight(); }src/app/shared/components/loading-spinner/loading-spinner.component.scss (1)
1-23: Missing:host { display: block; }per coding guidelines.The component SCSS should include
:host { display: block; }as required by the project's coding guidelines.Proposed fix
+:host { + display: block; +} + .spinner-border-lg { width: 3rem; height: 3rem; }As per coding guidelines,
**/*.component.scss: "Use:host { display: block; }in component SCSS and organize styles with section dividers".src/app/shared/components/loading-spinner/loading-spinner.component.ts (1)
10-11: Remove unusedstyleinput.The
styleinput declared on line 11 is never referenced in the template. The template hardcodes the class names"spinner-border spinner-border-lg loading-spinner"instead, and no consumers of the component pass this input. Consider removing it to eliminate dead code.
Resolve all open PR review threads for Techinterview-space#118.\n\n- Lazy-load Prism theme CSS with Prism runtime modules\n- Make Prism loader retry-safe after dynamic import failures\n- Catch non-critical Prism highlight promise rejections in call sites\n- Tighten navbar crawl-safe test by requiring login button render path\n- Switch first home feature image to eager loading for viewport/LCP safety\n- Bind loading spinner alt text to selected cat/dog asset\n- Prevent double-init timer leak in loading spinner spec\n\nValidation:\n- npx ng test --no-watch --browsers ChromiumNoSandbox --include=... (targeted)\n- npm run test-headless-ci-only\n- npm run build-prod
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/shared/services/prism-loader.service.ts`:
- Around line 71-72: The forEach callback in prism-loader.service.ts implicitly
returns a value because it uses an arrow expression body; change the callback to
a block body so it does not return anything: replace codeBlocks.forEach((block)
=> this.prism!.highlightElement(block)) with a block-bodied callback that calls
this.prism!.highlightElement(block); ensure you keep the same variables
(container, codeBlocks, block) and the method name highlightElement on
this.prism to locate the change.
🧹 Nitpick comments (2)
src/app/modules/interviews/components/interview-template-page/interview-template-page.component.ts (2)
96-102:queueMicrotaskmay fire before Angular finishes change detection.Microtasks are processed before rendering, so the DOM might not yet reflect the newly assigned
interviewTemplatewhen Prism tries to highlight. If you observe cases where highlighting is missed, switch tosetTimeout(macrotask) which runs after the rendering pipeline:private highlightCodeBlocks(): void { - queueMicrotask(() => { + setTimeout(() => { this.prismLoader .loadAndHighlight(this.elementRef.nativeElement) .catch(() => undefined); - }); + }); }Since tests are green today, this is not blocking — just something to watch for.
43-67: Pre-existing: deeply nested subscribes — consider refactoring withswitchMap.The triple-nested
.subscribe()chain makes reasoning about the data flow and teardown harder. This is pre-existing and not introduced by this PR, so no action needed now, butswitchMapchaining would simplify it.
Adjust Prism code block iteration callback to block-body form for style/lint clarity in the remaining unresolved PR review thread.
|
@maximgorbatyuk а можно провалидировать как-то без бека? Или может развернуть на стейдже? |
Summary
Implement Phase 1 SEO + performance improvements for
techinterview.spaceand/salarieswhile preserving product constraints:/salariesblocked from indexing/salaries/overviewas the SEO landing routeindex.htmlWhat changed
Crawlability and SEO hygiene
robots.txtcomments/order while keeping/salariesblocked and/salaries/overviewallowedGET /sitemap.xml->301tohttps://api.techinterview.space/api/sitemap.xml/salariesroute stabilityBundle/runtime payload reduction
AdminModule,InterviewsModule)angular.jsonMedia + CLS work
480/800) for key home visualsfetchpriority,decoding,srcset, andsizesto critical images/salariesto prevent footer jump during hydration/data loadingSSR delivery
Tests and verification
Automated
npm run test-headless-ci-only->211 SUCCESSAdded/updated test coverage
Infra Snapshot (captured 2026-02-10 11:58:50 UTC)
Live production observations (pre-deploy)
curl -sS -o /dev/null -w '%{http_version}' https://techinterview.space1.1curl -sS --http2 -o /dev/null -w '%{http_version}' https://techinterview.space1.1(HTTP/2 not negotiated)curl -sS --http2 -o /dev/null -D - https://techinterview.space --compressedContent-Encoding: gzipCache-Control: public, max-age=300, s-maxage=3600curl -sS -o /dev/null -D - https://techinterview.space/main-M5ERZKNV.js --compressedCache-Control: public, max-age=31536000Content-Encodingheader observedcurl -sS https://techinterview.space/robots.txt/salariesdisallowed;/salaries/overviewallowed; sitemap points to APIcurl -sSI https://techinterview.space/sitemap.xml200 OK+ HTML response (not redirect yet)curl -sS -o /dev/null -D - https://api.techinterview.space/api/sitemap.xml200 OK,Content-Type: application/xml,Cache-Control: public,max-age=3600wwwTLS certcurl -sSI https://www.techinterview.spaceCN/SANmismatch)wwwcert SAN detailsInfra Phase 2 checklist (outside this repo)
techinterview.space.wwwhost TLS/canonical behavior:www.techinterview.spacehttps://www.techinterview.space/*->https://techinterview.space/*/sitemap.xmlbehavior as301tohttps://api.techinterview.space/api/sitemap.xmlat both app and edge.Expected post-deploy checks
curl -I https://techinterview.space/sitemap.xml->301to API sitemap/salariesCLS should improve due reserved shell + stable media dimensionsSummary by CodeRabbit
New Features
Bug Fixes
Chores