Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
rdok opened this issue Mar 4, 2024 · 1 comment
Open
Labels
bug Something isn't working

Comments

@rdok
Copy link

rdok commented Mar 4, 2024

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 djhi added the bug Something isn't working label Mar 4, 2024
@djhi
Copy link
Collaborator

djhi commented Mar 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants