faro-web-sdk

faro-web-sdk copied to clipboard

Description

FaroRoute is breaking tests.

Steps to reproduce

1. Go to my sandbox
2. Hit the 'Test' tab on the right sidebar to run all tests
3. Observe the test failing

Expected behavior

The test should pass without breaking, when replacing Route with FaroRoute

Actual behavior

All tests with routing break.

This is what is displayed from the console when running tests which involve FaroRoute:

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

Check the render method of `FaroRoute`.

Environment

• SDK version: 1.0.0
• SDK instrumentations: React
• Device type: desktop
• Device name: MacBook Air M1 13"
• **OS:**MacOS
• **Browser:**Chrome

Demo

This can be reproduced here

Please note that on the project's package.json the specific versions of libs are the ones that I actually use on my project.

Hi @alioshr and thank you for trying out Faro.

In the example I can not find where Faro is initialized. Did you maybe forgot to add it?

faro-react - installation Add the Grafana Faro Web SDK package to your application

Cheers, Marco

Hi @codecapitano. Thanks so much for replying back.

I did not initialize it on this Demo as I just wanted to reproduce an error on an unit test. Even if I initialize faro, the unit test will fail in the same way. At least this is the scenario on my real project.

Is there any additional step on tests setup, or on the unit tests, to make FaroRoute work on tests environment? Am I missing anything? I could not find nothing related to testing on the docs.

Thanks in advance for your help!

Hey @alioshr I can reproduce the issue. It happens when FaroReact is not initialized. Anyways, I will dive a bit deeper an see what's going on.

Even if I initialize faro, the unit test will fail in the same way. At least this is the scenario on my real project.

Would you mind to add an example how you configured it?

Thanks, Marco

Hi @codecapitano. Thanks for replying back to me.

The initialization configuration of faro production code will have no influence over the unit tests unless something is needed to be done on test configuration setup / test file setup before running asserts.

Should we mock some faro internal code? I did not find documentation about this so far. My concern is that mocking things related to the Route itself could limit our testing capabilities.

Thanks in advance for taking a look into this.

This is how we init faro:

Just a note about FaroRoute regarding production code: While FaroRoute does not break the app on production code, we also are not able to get route changes metrics.

import {
  initializeFaro,
  getWebInstrumentations,
  ReactIntegration,
  ReactRouterVersion,
  Faro,
} from '@grafana/faro-react';
import { TracingInstrumentation } from '@grafana/faro-web-tracing';
import { Route, useHistory } from 'react-router-dom';

// .....

const faro = initializeFaro({
    url: 'some-url',
    apiKey: 'some-api-key',
    instrumentations: [
      ...getWebInstrumentations({
        captureConsole: true,
      }),
      new TracingInstrumentation({
        resourceAttributes: {
          'service.name': 'some-service-name',
          'team.name': 'some-team-name',
        },
      }),
      new ReactIntegration({
        router: {
          version: ReactRouterVersion.V5,
          dependencies: {
            history: useHistory,
            Route,
          },
        },
      }),
    ],
    session: (window as any).__PRELOADED_STATE__?.faro?.session,
    app: {
      name: 'some-app-name',
      version: 'some-app-version',
      environment: undefined,
    },
    user: {
      username: 'some-username',
    },
  });

Cheers,

Aliosh

Hi @alioshr

The Faro setup looks correct.

Regrading the testing issue: This is a bug and we've evaluated the place in code where it happens and will fix it.

Regarding the other issue you mentioned (we also are not able to get route changes metrics.): Would you open another issue for this? 🙏

Thanks you so much for reporting the issue(s).

Cheers, Marco

Hi @codecapitano ! Thanks for replying back to me and sorry for my late reply back to you.

Thanks for updating us regarding the potential bug on the tests environment. I will open a ticket for the production part of the integration =)

Do you think that the potential production issue could be related to the bug you have identified on the tests environment?

Cheers,

Aliosh

Thank you @alioshr 🙏 At the moment I can't say yet if the prod issue is related to the bug. I'll let you know as soon as I know more.

Cheers, Marco

Hi @alioshr

I had a look into the bug you have in production.

While FaroRoute does not break the app on production code, we also are not able to get route changes metrics.

From the provided example I see you are using ReactRouter.V5 and assign a reference to the useHistory hook to the history property in the router dependencies.

This will currently not work because we internally subscribe to history.listen and push route changes when the listener fires (it happens a bit more but this is roughly how it works). The problem is that it is not possible to call React hooks outside React Components.|

At the moment you need to create and pass the history object to the Faro React instrumentation. Note: For this to work you need to use the <Router /> component instead of <BrowserRouter />!

Example (stripped out things not related to the issue)

const history = createBrowserHistory();

initializeFaro({
  //... other options
  instrumentations: [
    //... other options
    new ReactIntegration({
      router: {
        version: ReactRouterVersion.V5,
        dependencies: {
          history: history,
          Route: Route,
        },
      },
    }),
  ],
});

//... other react init options

root.render(
    <Router history={history}>
      <App />
    </Router>
);

Cheers, Marco

@codecapitano thanks for the explanation!

I have managed to enable route changes metrics now. On the tests I still see everything breaking. My workaround was:

• Add an abstraction layer to have FaroRoute only on staging / Prod, with a fallback in dev to a normal Route (due to the fact the FaroRoute only works if faro is initialized), as we don't want to collect metrics in dev and our MicroFrontends use an external webapack version of @grafana/faro-react, so there is no initialization of faro in them
• Mock FaroRoute on tests in this single source of truth abstraction layer (a Component).

With this I have keep dev working and unit tests passing (will use normal Route in the tests)

My suggestions would be:

Make FaroRoute fallback to a regular Route with no api.push.... (be resilient) if no instance of faro is detected, as it would be good for dev experience and for using FaroRoute in test environment (we do not want to test faro, or stuff like this, but still want to assert on our route changes as usual)

Hi @alioshr

Nice, thank you for the feedback.

Regarding the testing case: We are currently working on a proposal which is exactly doing that. The current approach is to provide a React context which wraps around the app and gets the users provided Fallback Route as the value. The respective Faro components can receive the fallback route from the context if needed. Why not add <Route/> as a dependency? This is because we can not predict the react-router-dom version. Why no dynamic imports? Dynamic imports are relatively new browser feature and may not work for some browser.

I'll update you here.

Cheers, Marco