Default Values prevent warning

[johnathanludwig]

trafficstars

If you have a default value in your destructure that is an object or another variable then this will not raise a lint warning or attempt to fix.

Here is an example:

const {
  foo: 123,
  bar: 456,
  variable: someVar
} = someObj

Here are some test cases that recreate the issue:

{
  code: "const {a, b, c: {e, d, f = {}}} = someObj;",
  errors: [msg("e", "d")],
  output: "const {a, b, c: {d, e, f = {}}} = someObj;"
},
{
  code: "const {a, b, c: {e, d, f = foo}} = someObj;",
  errors: [msg("e", "d")],
  output: "const {a, b, c: {d, e, f = foo}} = someObj;"
},
{
  code: "const {a, b, c: {e, d, f = () => {})}} = someObj;",
  errors: [msg("e", "d")],
  output: "const {a, b, c: {d, e, f = () => {})}} = someObj;"
}

[xiaoshu-lin]

Hi @mthadley , I'm currently using this plugin and I notice this issue. I found you treat this kind of destructure as 'unsafe' to sort, I'm wodering what kind of issue this could cause? (value.type === "AssignmentPattern" && value.right.type === "Literal")

[mthadley]

Hey all, thanks for making a note about this. I'll try to explain some more.

One goal with the plugin is that it would never produce a warning (or auto-fix), that would change the behavior of the code. When it comes to sorting destructure properties, that's generally true, but there's an edge case with default values.

Testing with Node v12...

const food = {isVegetable: true};
const {isVegetable, isFruit = !isVegetable} = food;

console.log(isFruit); // `false`, like you would expect

Now, some code, but with the properties sorted by name:

const food = {isVegetable: true};
const {isFruit = !isVegetable, isVegetable} = food;

console.log(isFruit); // ReferenceError: Cannot access 'isVegetable' before initialization

Worth pointing out that when I originally wrote that check you mentioned @xiaoshu-lin, I remember seeing different behavior, which was that instead of ReferenceError, the default would instead become undefined. Not sure if my memory is bad, but regardless, it throws a runtime error now (which seems better).

So because of that, I decided to just avoid sorting if the default could possibly be an identifier that references another destructured property.

I think the rule could definitely be smarter though. It could traverse the AST in the destructure default value and explicitly check if there are any references that could be broken by sorting. However, I never implemented it, mostly because it seemed like a lot of work.

I'd love to hear other ideas, or if someone wants to take a shot at making the rule smarter here.

[reediculous456]

@mthadley I'm having this same issue, what if you make the rule that it doesn't throw an error when the default assignment uses a variable but it does throw the error if the assignment uses a literal?

For example:

Does not sort:

function get({ email, firstName, relations = someVar, role }) { }

Does sort:

function get({ email, firstName, relations = [ `roles` ], role }) { }

[patricklafrance]

It seems to work fine for literals, which is awesome. I wonder if it could also work for const var?

I have the following use case which doesn't get sorted:

const DefaultElement = "div";

.....

const {
    children,
    as = DefaultElement,
    ....
} = props;

Thank you,

Patrick

[vanyauhalin]

Keep watching. 👀

[mthadley]

I've published v1.5.0 which should hopefully allow many of these cases to now be sortable. If you run into any problems, please open a new issue and we'll sort it out there.

Shoutout to @ianobermiller for actual fix itself (#215).