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

feat: add codemod for is-plain-object #55

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

babs20
Copy link
Contributor

@babs20 babs20 commented Jul 22, 2024

Codemod for is-plain-object using a slightly modified replacement from the module-replacements repo. I left an issue over there to address an edge case I encountered while creating this codemod. I am 99% confident in my variation, which matches the package's original behavior.

@babs20 babs20 force-pushed the feature/is-plain-object branch from c001c07 to 430d87e Compare July 22, 2024 02:58
// Test cases
assert.strictEqual((typeof Object.create({}) === "object" && (Object.create({}).constructor === Object || Object.create({}).constructor === undefined)), true);
assert.strictEqual((typeof Object.create(Object.prototype) === "object" && (Object.create(Object.prototype).constructor === Object || Object.create(Object.prototype).constructor === undefined)), true);
assert.strictEqual((typeof { foo: 'bar' } === "object" && ({ foo: 'bar' }.constructor === Object || { foo: 'bar' }.constructor === undefined)), true);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cashing should be applied, for example, for object literals too. They could have some logic like computed properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I get what you mean. We should add some sort of caching into the codemod itself, right?

@babs20 babs20 force-pushed the feature/is-plain-object branch 2 times, most recently from 866805d to 27692b9 Compare July 23, 2024 21:21
@babs20
Copy link
Contributor Author

babs20 commented Jul 23, 2024

Updated this to:

  1. Use an IIFE so we can use a cached version of the value we are checking.
  2. Use the updated suggested replacement from the module-replacements repo

Also, it seems that there is a bit of different behavior from the is-plain-object package compared to the suggested replacement. For example:

const { isPlainObject } = require('is-plain-object')

const arg = Object.create({});
console.log(isPlainObject(arg));
//=> true
console.log(arg && typeof arg === "object" && (Object.getPrototypeOf(arg) === null || Object.getPrototypeOf(arg) === Object.prototype))
//=> false

It appears to me that the suggested replacement from the module-replacements repo creates the "correct" output, so maybe down the road as this package matures we will need to be clear that this codemod (and possibly some others) could change the behavior of the code they are replacing.

@babs20 babs20 force-pushed the feature/is-plain-object branch from 27692b9 to 77314b7 Compare July 23, 2024 22:55
@thepassle
Copy link
Collaborator

This LGTM, @zloirock what do you think?

Copy link

@zloirock zloirock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@thepassle thepassle merged commit a713374 into es-tooling:main Jul 24, 2024
3 checks passed
@thepassle
Copy link
Collaborator

Nice work and great collaboration guys!

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

Successfully merging this pull request may close these issues.

3 participants