Azure / react-appinsights

Typescript library to use Application Insights in React applications [deprecated, see microsoft/ApplicationInsights-JS]
MIT License
78 stars 32 forks source link

Update react-appinsights to application insights next version, update test, add ReactAIContainer to provide default container available to ReactAITracking #51

Closed jpiyali closed 5 years ago

jpiyali commented 5 years ago

Update react-appinsights to application insights next version, update test, add ReactAIContainer to provide default container available to ReactAITracking

Usage looks like the following: let reactAI = new ReactAI(); let appInsights = new ApplicationInsights({ config: { instrumentationKey: IKEY, extensions: [reactAI], extensionConfig: { "ApplicationInsightsReactUsage": { debug: false } } } }); ReactAIContainer.defaultReactAIContainer = new ReactAIContainer(appInsights, reactAI);

jpiyali commented 5 years ago

@hiraldesai - answering your question from my first PR (sent out this pr instead today). Usage:

let reactAI = new ReactAI(); let appInsights = new ApplicationInsights({ config: { instrumentationKey: IKEY, extensions: [reactAI], extensionConfig: { "ApplicationInsightsReactUsage": { debug: false } } } }); appInsights.loadAppInsights();

ReactAIContainer.defaultReactAIContainer = new ReactAIContainer(appInsights, reactAI);

The remaining is as before (withAITracking...). The intent of changing usage is so that customers can continue to incorporate other extensions into the SDK other than just react-appInsights as well.

jpiyali commented 5 years ago

@hiraldesai , @pviotti - any ideas on how to get rid of the build issue I'm having in the typescript-rewrite branch:

src/ReactAI.ts:73:33 - error TS2345: Argument of type 'History' is not assignable to parameter of type 'History'. Type 'History' is missing the following properties from type 'History': scrollRestoration, state, back, forward, and 2 more.

73 this.addHistoryListener(reactAISettings.history);


src/ReactAI.ts:122:13 - error TS2339: Property 'listen' does not exist on type 'History'.

122     history.listen(
pviotti commented 5 years ago

adding History to the import in ReactAI.ts fixes that problem:

import { Action, History, Location } from "history";

after that it looks like there are some other issues to fix to make the tests pass...

pviotti commented 5 years ago

/AzurePipelines help

azure-pipelines[bot] commented 5 years ago
Supported commands
     help:
          Get descriptions, examples and documentation about supported commands
          Example: help "command_name"
     run:
          Run all pipelines or a specific pipeline for this repository using a comment. Use
          this command by itself to trigger all related pipelines, or specify a pipeline
          to run.
          Example: "run" or "run pipeline_name"

See additional documentation.
pviotti commented 5 years ago

/AzurePipelines run

pviotti commented 5 years ago

In addition to what noted by Hiral, 4 tests out of 7 are not passing (and unfortunately for some reason the Pipeline CI build is not triggered on this PR..):

jpiyali commented 5 years ago

@hiraldesai , @pviotti , do you guys have any instructions for debugging the tests?

Update: @hiraldesai , @pviotti I've fixed all tests, added new ones. They were failing as I had missed calling loadAppInsights() for major part. Had to fix some other spy functions. All tests passing now. I'll be pushing this change in today. Please take a look if you have any other comments.