syncweek-react-aad / react-aad

A React wrapper for Azure AD using the Microsoft Authentication Library (MSAL). The easiest way to integrate AzureAD with your React for authentication.
MIT License
344 stars 94 forks source link

Test suite failed to run #201

Closed rosenjcb closed 4 years ago

rosenjcb commented 4 years ago

Library versions

Describe the bug When I run npm run test I get an error:

 ● Test suite failed to run

    ReferenceError: regeneratorRuntime is not defined

      30 | }
      31 | 
    > 32 | export const authProvider = new MsalAuthProvider(config, authenticationParameters, options)                                                  ^

Expected behavior When I run tests, I don't expect an error. My tests should just run successfully. I should note that failing tests are testing components which import the authProvider file. (I add some additional functionality in there to complete partially applied axios requests with bearer tokens from the acquireTokenSilent).

AndrewCraswell commented 4 years ago

I need to do more research on this. We're using async/await in the library, which will cause issues if you're Jest is not using the correct polyfills. I tend to avoid shipping polyfills, since it's the developer who determines which platforms the code needs to execute on. I can pacakage all the polyfills with the library, but it will add an additional 100kb to the build, and most of it will be unnecessary for the majority of clients.

Your best bet in the short term is to modify your Jest or Babel config to inject the regeneratorRuntime. This actually used to occur in Jest automatically, but was discontinued as described here: https://jestjs.io/blog/2019/01/25/jest-24-refreshing-polished-typescript-friendly#breaking-changes

Also see this issue for various resolutions: https://github.com/facebook/jest/issues/3126

But again, I need to do more research. It might make the most sense to just include this one polyfill. The library was recently migrated to Babel, so we have the ability to instruct it how to properly transpile await/async. I'm happy to accept a PR for this too, as I'll be out of the country for the next few weeks.

rosenjcb commented 4 years ago

@AndrewCraswell My google-fu was weak on this one. Kept running into problems with Babel and I was unable to figure out the issue other than it being related to async/await. I'll have to figure out how to do this. I don't even have a babel config (I just use CRA) so I'll have to figure out how that works.

FizzBuzz791 commented 4 years ago

I've hit this with a vanilla CRA and the solution was to add import "regenerator-runtime"; as the first line in my various test files that were failing. Not ideal, but I'm not sure how else to do it without ejecting and I don't want/need to do that at this point.

AndrewCraswell commented 4 years ago

Can't you just list regenerator-runtime in your Jest config under the setupFiles property? This should be able to be done from within the package.json file. Since it was Jest that was originally injecting this, it seems to make the most sense to manually ingest it via Jest.

For what it's worth, this is often what I do on other projects, because you'll find there are many other projects that don't ship with the regenerator runtime.

FizzBuzz791 commented 4 years ago

Yeah, I understand it's not actually this project that is broken. We should probably be discussing this in a CRA issue instead.

I don't think any of the available overrides support injection?

AndrewCraswell commented 4 years ago

I think this is the one you need, which isn't described in the CRA docs as far as I can tell, but I thought it could be used even without ejecting: https://jestjs.io/docs/en/configuration#setupfiles-array

FizzBuzz791 commented 4 years ago

Nah, no dice :(

image

AndrewCraswell commented 4 years ago

Hmmm other option is to use a setupTests file: https://create-react-app.dev/docs/running-tests/#srcsetuptestsjs-1

FizzBuzz791 commented 4 years ago

Yep, that's a much nicer solution than adding the import to each test that's affected. I wasn't aware of that, thanks!

brayanL commented 4 years ago

So, what is the solution? how should look like setupTests.js file?

FizzBuzz791 commented 4 years ago

Just add import "regenerator-runtime"; to the file and it should solve the problem.