Object rest includes non-enumerable Symbols

[planttheidea]

TypeScript Version: 3.2.4

Search Terms: object rest, rest symbols

Code

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.

const object = { visible: true };

Object.defineProperty(object, 'does not appear', {
  enumerable: false,
  value: 'hidden key',
});

Object.defineProperty(object, Symbol('appears on rest'), {
  enumerable: false,
  value: 'hidden symbol',
});

// does not include non-enumerable symbol or key
console.log({ ...object });

const { visible, ...rest } = object;

// includes non-enumerable symbol, but excludes key
console.log(rest);

Expected behavior:

Non-enumerable symbols are not include in Object rest operations.

Actual behavior:

Non-enumerable symbols are included in Object rest operations.

Playground Link:

Playground

Related Issues:

I believe this was introduced with this PR => #12248.

This included Symbols in rest, but uses Object.getOwnPropertySymbols without using Object.prototype.propertyIsEnumerable check in the loops. You can see this in the generated output in the playground above.

[NicholasLYang]

I decided to give this a crack. I added an Object.prototype.propertyIsEnumerable check to__rest in destructuring.ts.

I attempted to fix the baseline tests, but I'm not entirely sure about how to do that. Should I just change the reference files manually? Or is there a better way of doing that? @weswigham

[weswigham]

Run a difftool over the directories and use the tool to copy over the changes (if you have a DIFFTOOL environment variable specified, jake diff should launch it with the correct folders). Failing that, jake baseline-accept should just copy over them all.

[NicholasLYang]

Great, thanks! I pushed all my test changes as well. Should I open a PR?

[NicholasLYang]

@weswigham, just wondering if there's anything else that needs to be done. I opened a PR for the fix.