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

this.files being populated in a bad order #1338

Open
diosney opened this issue May 22, 2015 · 6 comments
Open

this.files being populated in a bad order #1338

diosney opened this issue May 22, 2015 · 6 comments
Labels

Comments

@diosney
Copy link

diosney commented May 22, 2015

Right now this.files is being populated as:

  'src/applications/hr/admin/organization/settings/settings.js',
  'src/applications/hr/admin/organization/settings/settings-controller.js',
  'src/applications/hr/hr.js',
  'src/applications/hr/hr-services.js'

and should be like:


  'src/applications/hr/hr.js',
  'src/applications/hr/hr-services.js'
  'src/applications/hr/admin/organization/settings/settings.js',
  'src/applications/hr/admin/organization/settings/settings-controller.js',

This seems to be a minor issue, but when used with plugins like grunt-contrib-concat lead to wrong concatenated files, since the code execution order is mangled.

@diosney
Copy link
Author

diosney commented May 22, 2015

Something like https://github.com/hughsk/path-sort should be used to sort the files array and not use the native Array.prototype.sort method.

@jonschlinkert
Copy link

can you link to the offending line of code so we can discuss? thanks!

@cowboy
Copy link
Member

cowboy commented May 23, 2015

@diosney the current sorting behavior is working as-expected. Unfortunately, Grunt doesn't currently allow for a custom sorting method, but perhaps that could be added in a future version; it seems like a useful addition.

Could you be more explicit in how you'd like the sorting behavior to change, using words instead of an example, why you'd like it to change, and why you think it's a bug?

The only thing I can infer from your example (and from the path-sort readme) is that you want files to come before directories, but after that, for files and directories to be sorted lexicographically. But I'm not comfortable making an assumption here, so I'd like you to clarify.

Thanks!

@diosney
Copy link
Author

diosney commented May 24, 2015

@jonschlinkert @cowboy Thanks for your quick response, I will expand below with both words and examples.

The current sorting behavior it seems that use the native Array.prototype.sort method, which does a string per-character comparison, so the paths src/applications/hr/admin/organization/settings/settings-controller.js and src/applications/hr/hr.js, become sorted as:

  //                 here
  //                  |
 'src/applications/hr/admin/organization/settings/settings-controller.js',
 'src/applications/hr/hr.js',

which are "correctly" sorted using the alphanumeric sorting order explained above since the a of admin is lower than the h of hr.

The issue with this algorithm is that hr.js-like files, usually declares functionality that it needs to be loaded first than the other files in its subdirectory, but due the ordering is executing at a later time, triggering a not-found-like error. Angular apps are an example of this situation.

Now, think of my current situation here, I have nearly 300 files that need to be concatenated (the uglify task has this issue too, of course) and for them to work properly I had to manually specify its ordering in the concat task, which is cumbersome and can be easily avoided if the pattern I used worked as expected.

If instead of that, a path-sorting function is used on this cases as you,@cowboy, correctly assumed 😄, this kind of bugs in app code will likely disappear.

Only one caveat to this, I do want that the sorting algorithm change for the wildcard patterns like src/**, but not for the patterns that defines static files, like the following:

src: [
    'src/z.js',
    'src/a.js
]

since when they are specified is because their specified ordering matters, so I recommend that the path-sorting algorithm is specified only for wildcard file patterns only.

I hope this makes sense for you, if not, just tell me I will expand further.

Thanks

@jonschlinkert
Copy link

The current sorting behavior it seems that use the native Array.prototype.sort method

@cowboy aside from glob, where else in Grunt might sorting be used on files currently?I didn't think it was, maybe I looked right at the code and missed it...

perhaps that could be added in a future version; it seems like a useful addition.

agreed, it does sound like a nice feature to have.

@diosney
Copy link
Author

diosney commented May 24, 2015

@jonschlinkert Thanks for your comment.

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

No branches or pull requests

4 participants