marmelab / ra-auth-cognito

An auth provider for react-admin which handles authentication with AWS Cognito.
MIT License
9 stars 6 forks source link

Breaking minor changes: TOTP MFA: requires additional external dependency #16

Open rdok opened 6 months ago

rdok commented 6 months ago

Replication

Upgrade an react-admin application's ra-auth-cognito from v1.0.2 to to latest minor version (v1.1.0 at the time of this writing)

  "dependencies": {
    "amazon-cognito-identity-js": "^6.3.7",
    "ra-auth-cognito": "^1.1.0",
    "ra-data-graphql-simple": "^4.16.12",
    "react": "^18.2.0",
    "react-admin": "^4.16.12",
    "react-dom": "^18.2.0",
    "react-router-dom": "^6.22.2"
  }

Caused By

Error

$ vite --clearScreen false

  VITE v4.5.2  ready in 146 ms

  ➜  Local:   http://localhost:5173/
  ➜  Network: http://192.168.0.111:5173/
  ➜  Network: http://172.27.0.1:5173/
  ➜  Network: http://172.26.0.1:5173/
  ➜  Network: http://172.25.0.1:5173/
  ➜  Network: http://172.20.0.1:5173/
  ➜  Network: http://172.18.0.1:5173/
  ➜  Network: http://172.28.0.1:5173/
  ➜  Network: http://172.23.0.1:5173/
  ➜  press h to show help
✘ [ERROR] Could not resolve "qrcode.react"

    node_modules/ra-auth-cognito/esm/MfaTotpAssociationForm.js:3:26:
      3 │ import { QRCodeSVG } from 'qrcode.react';
        ╵                           ~~~~~~~~~~~~~~

  You can mark the path "qrcode.react" as external to exclude it from the bundle, which will remove
  this error.

error Command failed with exit code 1.

Expected Behaviour

The TOTP MFA should only error upon usage of said feature; or any similar approach. Although installing qrcode.react also resolves this feature, I don't see why clients should be forced to install this when they are not using it; previously the ra-core was required; which was an acceptable peer dependency as it was an marmelab in-house library (was was filled indirectly by react-admin, as opposed to the external qrcode.react library. For now, I'll just hold the version back to v1.0.2, until marmelab developer provide guidance.

If the qrcode.react has become a required peer dependency, the docs should be updated accordingly. Optimally with relevant changelog for handling these breaking changes.

djhi commented 6 months ago

That's an issue indeed. Thanks for reporting it!