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

Issue with getChangelist, transition.queryParams getting mutated by stringified #340

Open
betocantu93 opened this issue Oct 23, 2024 · 1 comment

Comments

@betocantu93
Copy link

betocantu93 commented Oct 23, 2024

Issue with getChangelist in Router.js

In Ember, while transitioning between routes, the function getChangelist is used to compare the state between transitions. You can find its implementation here:

The problem with this function is that, when comparing old and new queryParams, it mutates the parameters by stringifying them, which I believe should be immutable. The issue arises from this code:

For example, if the query parameters contain an array of objects, the result becomes ['[object Object]']. A current workaround to this problem is to avoid using arrays as values, as it wont get stringified by lack of cases in the stringifier function

Complex QueryParams Example:

In our Ember app, we use a complex structure for queryParams when transitioning between routes. For instance:

router.transitionTo('some.route', {
   queryParams: {
      reports: [
        {
         someData: 10,
        }
      ]
   }
})

However, we don't really map all this data to the URL, some of these is read and thats it.

class SomeRoute extends Route {
  model(_, transition) {
     const { to } = transition;

     // Here, reports is an array of ['object object']
     transition.to.reports.forEach(); // This will has an issue, all of the elements are now `['[object Object]']`
  }
}

Observations:

  1. Immutability: The parameters shouldn't be mutated at all, especially since it's only for comparison purposes. An alternative approach should be used.

  2. Comparison Logic: When comparing objects, a more robust library like deepEquals or fastEquals should be used? Because there are too many edge cases that can't be properly handled by simply coercing values to strings.

@betocantu93 betocantu93 changed the title Why does the transition queryParams values get stringified Issue with getChangelist in Router.js Oct 23, 2024
@betocantu93 betocantu93 changed the title Issue with getChangelist in Router.js Issue with getChangelist Oct 23, 2024
@betocantu93 betocantu93 changed the title Issue with getChangelist Issue with getChangelist, queryParams getting stringified Oct 23, 2024
@betocantu93 betocantu93 changed the title Issue with getChangelist, queryParams getting stringified Issue with getChangelist, transition.queryParams getting mutated by stringified Oct 23, 2024
@betocantu93
Copy link
Author

betocantu93 commented Oct 23, 2024

Not only that, it mutates and coerce all sort of values

queryParams: { hola: 5 } => { hola: '5' }
queryParams: { hola: [true, false] } => { hola: ['true', 'false'] }
queryParams: { hola: [{obj: 1}] } => { hola: ['object Object'] }

This is so random, like why only numbers? and not booleans? so many questions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant