require that inputs be plain objects (or arrays of)

[yutak23]

This resolves the issue #121.

I am updating the README to match the TypeScript type definitions.

Then update any[] to ReadonlyArray<Record<string, unknown>> to accept only the key-value record type of the array.

[yutak23]

I have removed the unsupported array format test.

[yutak23]

@bendrucker, I would appreciate your confirmation.

[bendrucker]

Please go ahead and separate the documentation and type concerns since I'm left unsure as to whether there's any legitimate issue here.

I do not put TypeScript in readmes as ultimately these are intended as JavaScript packages and should meet that lowest common denominator. The types given are JS pseudo-code. I do accept PRs with user-contributed types but TypeScript fluency should not be required to read the docs.

If you want to drop support for arrays you'll need to do more to make the case for that breaking change and make the change comprehensively. Obviously it's a bit silly to try to snake case an array of strings. Presumably this was a possible path because arrays are objects in JS and could be passed to map-obj without erroring. Dropping support for arrays also means the JavaScript code should also reject them.

[bendrucker]

This is an informal pseudo-JS notation. I am not trying to use TypeScript types here. See exclude for example.

[bendrucker]

Happened to be peeking at map-obj for another PR

sindresorhus/map-obj#39

This happened well after I started using map-obj. Sindre went with a patch. This is one of the many ambiguous semver cases where it's strictly breaking but you can surely argue that "this wasn't supposed to work."

I also haven't updated map-obj since v5 is ESM only though, so you'd still have to implement the array checking here until/if I get around to an ESM conversion that can apply the patch to the aforementioned issue.

[yutak23]

Please go ahead and separate the documentation and type concerns since I'm left unsure as to whether there's any legitimate issue here.

I do not put TypeScript in readmes as ultimately these are intended as JavaScript packages and should meet that lowest common denominator. The types given are JS pseudo-code. I do accept PRs with user-contributed types but TypeScript fluency should not be required to read the docs.

If you want to drop support for arrays you'll need to do more to make the case for that breaking change and make the change comprehensively. Obviously it's a bit silly to try to snake case an array of strings. Presumably this was a possible path because arrays are objects in JS and could be passed to map-obj without erroring. Dropping support for arrays also means the JavaScript code should also reject them.

Happened to be peeking at map-obj for another PR

sindresorhus/map-obj#39

This happened well after I started using map-obj. Sindre went with a patch. This is one of the many ambiguous semver cases where it's strictly breaking but you can surely argue that "this wasn't supposed to work."

I also haven't updated map-obj since v5 is ESM only though, so you'd still have to implement the array checking here until/if I get around to an ESM conversion that can apply the patch to the aforementioned issue.

@bendrucker , Thank you for your review. Are you sure that you want to follow the following steps to correct this issue?

1. create a PR with only TypeScript type definitions
2. modify the implementation to support arrays in the README and JS code (support only plain objects)

[bendrucker]

Yeah, let's do this in multiple distinct steps if possible for clarity. If there's any changes you think should be made right now to the TypeScript definitions, based on what the JS code supports, please send that as one PR. If the TS already accurately reflects the inputs that the underlying JS implementation supports, no change is needed here.

Separately, you can send another PR dropping support for arrays, which will involve both TS and JS changes. The TS definition can make arrays a compile error, but also the JS should reject arrays with a TypeError to ensure uniform behavior. Perhaps I should have waited to release v6 and included this, but I didn't think of it. Given the presence of a type test I'd probably err on the side of v7 rather than 6.0.1.

[yutak23]

Yeah, let's do this in multiple distinct steps if possible for clarity. If there's any changes you think should be made right now to the TypeScript definitions, based on what the JS code supports, please send that as one PR. If the TS already accurately reflects the inputs that the underlying JS implementation supports, no change is needed here.

Separately, you can send another PR dropping support for arrays, which will involve both TS and JS changes. The TS definition can make arrays a compile error, but also the JS should reject arrays with a TypeError to ensure uniform behavior. Perhaps I should have waited to release v6 and included this, but I didn't think of it. Given the presence of a type test I'd probably err on the side of v7 rather than 6.0.1.

@bendrucker, sorry for the late reply, I have changed the PR to fix the TS type definitions as a set with the JS code fixes.
I would appreciate your confirmation.

[bendrucker]

Thanks for your work on this! Out of caution I'm going to release this as a new major version since both the JS and typing changes have potential to break people's code.

[hhanesand]

I think this may have introduced a bug with arrays. This test no longer functions, because it throws Error: obj must be an plain object

it("should return array of snake_case objects", function () {
  const test = [{ camelCase: "foo" }];
  expect(snake(test)).to.deep.equal([{ camel_case: "foo" }]);
});

Most likely the check for the array input type should be

if (is array) {
   assert array contents
} else {
   assert is object
}