polyfact / polyfire-js

🔥 React library of AI components 🔥
https://www.npmjs.com/package/polyfire-js
133 stars 8 forks source link

fix: cleaning the URL properly #136

Closed victorforissier closed 8 months ago

victorforissier commented 8 months ago

Doing this pull request because the handling of the access_token on auth breaks user experience. When there are other useful params in the path (e.g. ?docId=xxxxx) the cleanup in the code removes everything instead of just the access_token & refresh_token.

I fixed it by splitting the path and removing just the access_tokens and refresh_tokens and then rebuilding it.

lowczarc commented 8 months ago

Also, why do you define an anonymous function to call it right after ?

victorforissier commented 8 months ago
  1. Yes makes sense. Like that?
  2. Wrapped it inside a function because I think it's clearer and delimits that specific action.
lowczarc commented 8 months ago

This is reallocating an anonymous function at runtime every time you're executing it, and it's inconsistent with the rest of the code, pls define your function at global level (and with the function keyword instead of const = () =>...)

lowczarc commented 8 months ago

Also the function name is backward

victorforissier commented 8 months ago

Your memory tests are broken.

lowczarc commented 8 months ago

Your memory tests are broken.

Fixed on main. Don't mind it