Skip to content

Conversation

@slifty
Copy link
Contributor

@slifty slifty commented Jan 16, 2026

This PR fixes a bug that was causing the info pane to fail to load after a recent change to the way we loaded NewRelic (which, apparently, is unrelated to NewRelic itself)

An analysis of the situation, ala Claude:


Why NewRelic Exposed the Bug

The root cause was always a circular dependency between EditTagsComponent → DataService → (via EditService) → EditTagsComponent. This was a latent bug that existed before PR #887 but never manifested.

The Chain of Events

Before PR #887:

  • NewRelic was loaded inline in index.html as a <script> tag
  • It executed completely before Angular's bundle loaded
  • The bundler (Webpack/Vite) had a specific module evaluation order
  • By coincidence, data.service.ts was always fully evaluated before edit-tags.component.ts

After PR #887:
// main.ts - BEFORE Angular imports
import { BrowserAgent } from '@newrelic/browser-agent/loaders/browser-agent';
import { AppModule } from './app/app.module'; // Angular loads after

Adding @newrelic/browser-agent as a static import at the top of main.ts caused the bundler to:

  1. Re-analyze the entire dependency graph
  2. Reorganize which modules go into which chunks
  3. Change the module evaluation order

The new order happened to evaluate edit-tags.component.ts before data.service.ts was fully initialized, causing DataService to be undefined when Angular tried to process the providers array.

Why Removing NewRelic Didn't Fix It

Once the bundler configuration changed (new index.html, new main.ts structure), the module evaluation order was permanently altered. NewRelic was just the trigger - removing it didn't restore the old order.

The Actual Fix

Removing DataService from EditTagsComponent's component-level providers eliminated the need for DataService to be defined at module decoration time, breaking the circular dependency.

@codecov
Copy link

codecov bot commented Jan 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.76%. Comparing base (1d9ba0f) to head (ce15770).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #898      +/-   ##
==========================================
- Coverage   47.77%   47.76%   -0.01%     
==========================================
  Files         351      351              
  Lines       11292    11292              
  Branches     1889     1889              
==========================================
- Hits         5395     5394       -1     
- Misses       5703     5709       +6     
+ Partials      194      189       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@slifty slifty force-pushed the noissue-fix-info branch 3 times, most recently from 66fb1fe to 9f67498 Compare January 16, 2026 20:57
@slifty slifty changed the title WIP: Fix NewRelic Fix the Info Pane Jan 16, 2026
@slifty slifty force-pushed the noissue-fix-info branch 2 times, most recently from 73c6f4d to 24f1bab Compare January 16, 2026 21:03
A recent change to our index.html seems to have exposed a latent bug
related to a circular dependency definition in our tag editor component.

NewRelic is being loaded in a new way, which apparently changed the
order that Angular was loading dependencies such that this became
exposed.

This also updates our DataService mock in the relevant test to account
for an attribute that was missing / incomplete in a way that didn't
matter before because it wasn't actually being used.
@slifty slifty marked this pull request as ready for review January 16, 2026 21:05
Copy link
Member

@cecilia-donnelly cecilia-donnelly left a comment

Choose a reason for hiding this comment

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

Thanks for figuring this out! It looks like it's working now. 🎉

@slifty slifty merged commit 95fdf2a into main Jan 17, 2026
12 of 13 checks passed
@slifty slifty deleted the noissue-fix-info branch January 17, 2026 02:52
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.

3 participants