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.

Sign up for GitHub

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

Jump to bottom

fix: avoid legacy accessors #2013

Merged
merged 8 commits into from Oct 7, 2021
Merged

fix: avoid legacy accessors #2013

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
289ec49
Avoid legacy accessors
bmeck Aug 22, 2021
f2d55b0
Merge branch 'master' into legacy-object-proto-accessors
bcoe Aug 25, 2021
0bcac69
Update index.cjs
bmeck Aug 25, 2021
6026e15
handle prototype based accessors
bmeck Aug 25, 2021
0812823
Merge branch 'master' into legacy-object-proto-accessors
bcoe Sep 5, 2021
313dda6
Merge branch 'master' into legacy-object-proto-accessors
bcoe Sep 9, 2021
96ff2c5
test: increase coverage
bcoe Oct 7, 2021
ea85e82
Merge branch 'main' into legacy-object-proto-accessors
bcoe Oct 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 23 additions & 7 deletions index.cjs
View file
Open in desktop
Expand Up @@ -15,6 +15,26 @@ function Argv(processArgs, cwd) {
return argv;
}

function defineGetter(obj, key, getter) {
Object.defineProperty(obj, key, {
configurable: true,
enumerable: true,
get: getter,
});
}
function lookupGetter(obj, key) {
while (obj) {
const desc = Object.getOwnPropertyDescriptor(obj, key);
if (typeof desc !== 'undefined') {
if (desc.get) {
return desc.get;
Copy link
Member

Choose a reason for hiding this comment

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

@bmeck can we replace this with:

function lookupGetter(obj, key) {
  const desc = Object.getOwnPropertyDescriptor(obj, key);
  if (typeof desc !== 'undefined') {
    return desc.get;
  }
}

So that coverage remains at 100%? I'm assuming that desc.get will either be the getter function, or undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did a pass to see if it was possible to go through a proto and it only occurs on extremely exotic and tailored Proxy stuff so this replacement should be fine due to it being invoked against the keys/own prop keys.

}
return undefined;
}
obj = Object.getPrototypeOf(obj);
}
}

/* Hack an instance of Argv with process.argv into Argv
so people can do
require('yargs')(['--beeble=1','-z','zizzle']).argv
Expand All @@ -28,16 +48,12 @@ function singletonify(inst) {
...Object.getOwnPropertyNames(inst.constructor.prototype),
].forEach(key => {
if (key === 'argv') {
Argv.__defineGetter__(key, inst.__lookupGetter__(key));
defineGetter(Argv, key, lookupGetter(inst, key));
} else if (typeof inst[key] === 'function') {
Argv[key] = inst[key].bind(inst);
} else {
Argv.__defineGetter__('$0', () => {
return inst.$0;
});
Argv.__defineGetter__('parsed', () => {
return inst.parsed;
});
defineGetter(Argv, '$0', () => inst.$0);
defineGetter(Argv, 'parsed', () => inst.parsed);
}
});
}