microsoft / ApplicationInsights-JS

Microsoft Application Insights SDK for JavaScript
MIT License
646 stars 238 forks source link

[BUG] window.appInsights is `undefined` #1951

Open Snailedlt opened 1 year ago

Snailedlt commented 1 year ago

Edit

So apparantly this isn't a bug, but it's still very confusing since it doesn't work the way that's described in the deocumentation. The documentation needs to be updated to mention that the npm version doesn't populate window.appInsights, so it needs to be exported to be used in other components. See this comment below for an example: https://github.com/microsoft/ApplicationInsights-JS/issues/1951#issuecomment-1329154828

Original description

Description/Screenshot window.appInsights is undefined, so I'm unable to use any appInsights methods in other files than the one where I declared appInsights. I'm using Svelte if that matters, and appInsights is initialized in App.svelte

in vscode: image

in google chrome: image

code:

    const appInsights = new ApplicationInsights({
      config: {
        connectionString:
          process.env.AZURE_APPLICATION_INNSIGHTS_CONNECTION_STRING,
      },
    });
    appInsights.loadAppInsights();
    appInsights.trackPageView(); // Manually call trackPageView to establish the current user/session/pageview

    window.appInsights.trackEvent('Test');

error messages:

Property 'appInsights' does not exist on type 'Window & typeof globalThis'.ts(2339)

App.svelte:29 Uncaught TypeError: Cannot read properties of undefined (reading 'trackEvent') at instance (App.svelte:29:24) at init (index.mjs:1997:11) at new App (App.svelte:45:40) at main.js:3:13 at main.js:5:2

Steps to Reproduce

Expected behavior window.appInsights is NOT undefined

Additional context Add any other context about the problem here.

Karlie-777 commented 1 year ago

@Snailedlt if I am understanding it correctly, you are using npm(click here for details) to initialize ApplicationInsights. So under this scenario, ApplicationInsights is not global. But if you are using snippet initialization (click here for details), you can access window.appInsights directly

Snailedlt commented 1 year ago

@Karlie-777 yes, that is correct, I'm using the npm version. I don't see anything in the documentation saying it's not global. How would I go about accessing it globally, and what's the reason it's not global in the first place?

My usecase is that I want to call trackEvent from multiple different files, should I be using the snippet based initialisation for rhat instead?

Karlie-777 commented 1 year ago

@Snailedlt so if you are using npm initialization, you can create an ApplicationInsights component ( see example here ). If you want to easily use window.appInsights, snippet initialization is recommended.

Snailedlt commented 1 year ago

@Karlie-777 I see, thank you! So if I use the snippet initialization, do I just need to initialize it once, and then I can access it through window.appInsights after that, or do I need to initialize it in every file I want to use it?

Karlie-777 commented 1 year ago

@Snailedlt snippet initialization creates global instance. so as long as you can access window object, you can access window.app Insights directly!

Snailedlt commented 1 year ago

Okey, so what I'm getting is... If I want to use it in one file only, go for npm, otherwise go for the snippet?

Karlie-777 commented 1 year ago

@Snailedlt depends on your website structure. For npm, if you creates applicationInsights component and it can be accessed by all files also.

Snailedlt commented 1 year ago

So when I try to use the snippet based version I need to initialize it as early as possible. I tried pasting this into the index.html file of our app. I only replaced the connectionString with process.env.AZURE_APPLICATION_INNSIGHTS_CONNECTION_STRING from our .env file. But I get an error saying ReferenceError: “process is not defined”. I assume the error comes because the file is accessed before the .env is initialized, so process is undefined at that moment.

Gonna try putting the npm snippet into a component like you suggested earlier.

Snailedlt commented 1 year ago

I ended up just exporting appInsights like so:

import { ApplicationInsights } from '@microsoft/applicationinsights-web';

export const appInsights = new ApplicationInsights({
  config: {
    connectionString: process.env.AZURE_APPLICATION_INNSIGHTS_CONNECTION_STRING,
  },
});

And then loading it in App.svelte

  import { appInsights } from './util/appInsights';

  appInsights.loadAppInsights();
  appInsights.trackPageView(); // Manually call trackPageView to establish the current user/session/pageview

And finally using it in any other file by importing it:

  import { appInsights } from '../util/appInsights';
  ...
  ...
    // Log to Azure Application Insights
    if (
      process.env.isProd ||
      process.env.AZURE_APPLICATION_INNSIGHTS_CONNECTION_STRING
    ) {
      appInsights.trackEvent(
        {
          name: `FeatureForm${featureName}`,
        },
        {
          feature: {
            featureName: featureName,
            featureDescription: description,
          },
          userInput: {
            userHasOpinion: userHasOpinion,
            selectedAnswer: selectedAnswer,
            text: userText,
            email: userEmail,
          },
        }
      );
    } else {
      console.log('Application Insights is NOT active!');
    }
Karlie-777 commented 1 year ago

@Snailedlt

I only replaced the connectionString with process.env.AZURE_APPLICATION_INNSIGHTS_CONNECTION_STRING from our .env file. But I get an error saying ReferenceError: “process is not defined”. I assume the error comes because the file is accessed before the .env is initialized, so process is undefined at that moment.

for many website structures, process.env can't be referenced directly in index.html, for example, if you are using React, you have to use env variables like this: connectionString: `%REACT_APP_CONNECTION_STRING%`. so using env variables in correct ways might help.

Snailedlt commented 1 year ago

Nice, thanks!

It works now, so I edited the description to reflect the issue with misinformational documentation

krishnasaga commented 1 year ago

Nice, thanks!

It works now, so I edited the description to reflect the issue with misinformational documentation

I hope you have managed to resolve your issue.

Regarding your concern, I would like to mention that I don't consider it a problem with the documentation or a bug. The current documentation assumes that readers have a certain level of familiarity with JavaScript in browsers. However, I understand that your question revolves more around the topic of scoping in JavaScript. Adding such detailed information to the documentation may result in it resembling more of a comprehensive JavaScript book rather than a focused technical documentation specifically for this SDK.

Snailedlt commented 1 year ago

@Karlie-777 Thanks. We ended up just making a utility function that gets the instance :)