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

Pick could reorder #418

Open
mariusrak opened this issue Dec 3, 2020 · 6 comments
Open

Pick could reorder #418

mariusrak opened this issue Dec 3, 2020 · 6 comments

Comments

@mariusrak
Copy link

Hi, if I use .pick() on schema it could reorder fields by order given in the pick call not by original order.

@github-actions
Copy link

github-actions bot commented Dec 3, 2020

Thank you for submitting an issue!

If this is a bug report, please be sure to include, at minimum, example code showing a small schema and any necessary calls with all their arguments, which will reproduce the issue. Even better, you can link to a saved online code editor example, where anyone can immediately run the code and see the issue.

If you are requesting a feature, include a code example of how you imagine it working if it were implemented.

If you need to edit your issue description, click the [...] and choose Edit.

Be patient. This is a free and freely licensed package that I maintain in my spare time. You may get a response in a day, but it could also take a month. If you benefit from this package and would like to see more of my time devoted to it, you can help by sponsoring.

@aldeed
Copy link
Collaborator

aldeed commented Dec 4, 2020

@mariusrak Have you checked to see whether other pick functions such as lodash preserve the order?

Also, JavaScript prior to ES2015 doesn't guarantee object key iteration order for any objects, and even with ES2015 it is not 100% guaranteed. So I generally wouldn't rely on it.

That being said, a few aspects of SimpleSchema do act differently depending on key order, and in general I believe a schema object would always have a guaranteed key iteration order with ES2015.

I am not sure whether I think this should be "fixed", though. If pick were updated to keep the same order, it would be impossible to pick keys in a different order. Right now, you can pick in the same order or a new order by choosing how you order the key names in the array. So maybe this just needs to be documented but not changed?

@mariusrak
Copy link
Author

I'm not sure whether I understand. I wanted to pick keys in different order, but even when I provided keys in order different than in original schema, they were ordered by original schema not by new order.

@aldeed
Copy link
Collaborator

aldeed commented Dec 5, 2020

Oh I see. Your description sounded like a bug report but you are requesting a feature.

@mariusrak
Copy link
Author

Well originally I thought it is a feature request. But you wrote

Right now, you can pick in the same order or a new order by choosing how you order the key names in the array. So maybe this just needs to be documented but not changed?

So I'm not sure how it should work. Maybe I'm just using it wrong. I use the simple-schema with uniforms package. I have schema from fields e.g.

{field1: String, field2: String, field3: String}

and I do

schema.pick('field3', 'field1')

I expect to see form with field3 first and field1 second. But they are ordered as was originally defined. This contradicts your claim. So I guess we did not understood each other?

@aldeed
Copy link
Collaborator

aldeed commented Dec 10, 2020

@mariusrak No I wrote that from memory without checking. It does preserve the original order. I think the best solution to avoid any breaking changes would be to keep pick as is and add a pickInNewOrder (or whatever name people like) method that does what you are asking for.

In the meantime, you could write your own with something like this:

function pickInNewOrder(schema, ...fields) {
  const newSchema = {};

  for (const field of fields) {
    for (const key of this._schemaKeys) {
      if (key === field || key.indexOf(`${field}.`) === 0) {
        newSchema[key] = this._schema[key];
      }
    }
  }

  return schema._copyWithSchema(newSchema);
}

And call it like:

const newSchema = pickInNewOrder(schema, 'field3', 'field1')

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

No branches or pull requests

2 participants