Skip to content

Make CacheLocation an enum #848

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

Closed
5 tasks
jodonnell opened this issue Jul 19, 2019 · 3 comments
Closed
5 tasks

Make CacheLocation an enum #848

jodonnell opened this issue Jul 19, 2019 · 3 comments

Comments

@jodonnell
Copy link
Contributor

jodonnell commented Jul 19, 2019

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Browser:

  • Chrome version XX
  • Firefox version XX
  • IE version XX
  • Edge version XX
  • Safari version XX

Library version


Library version: X.Y.Z

Current behavior

when setting cacheLocation with typescript the user becomes victim to type widening so something like this is required

  const config = {
    auth: {
      authority: (process.env.REACT_APP_AUTHORITY as string),
      clientId: (process.env.REACT_APP_AAD_APP_CLIENT_ID as string),
    },
    cache: {
      cacheLocation: ('localStorage' as CacheLocation),
      storeAuthStateInCookie: true,
    },
  };

Expected behavior

I think it would be a bit clearer if CacheLocation was an enum so that may be used directly without type widening

enum CacheStorage {
  LocalStorage = "localStorage",
  SessionStorage = "sessionStorage"
}
........
  const config = {
    auth: {
      authority: (process.env.REACT_APP_AUTHORITY as string),
      clientId: (process.env.REACT_APP_AAD_APP_CLIENT_ID as string),
    },
    cache: {
      cacheLocation: CacheStorage.LocalStorage,
      storeAuthStateInCookie: true,
    },
  };

See discussion here: syncweek-react-aad/react-aad#95 (comment)

Minimal reproduction of the problem with instructions

@sameerag
Copy link
Member

Sure. @jodonnell Are you open to submitting a PR for the same?

@jodonnell
Copy link
Contributor Author

#851

i was only sort of able to test this in context so i'm not 100% confident it works like i want it to :'(

@sameerag
Copy link
Member

Merged this to dev, @jodonnell Please let us know if you need anything else in this context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants