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: proxy-less env support #171

Closed
wants to merge 6 commits into from
Closed

fix: proxy-less env support #171

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
311c075
Proxy-less env support
bassarisse Nov 7, 2018
65eb8df
chore: build
May 11, 2020
38fe004
chore: remove redundant repetition
May 12, 2020
3a6255e
chore: readability
May 12, 2020
6a34e9a
chore: explicit return
May 14, 2020
57249c9
chore: addRuleSet return
May 14, 2020
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
28 changes: 27 additions & 1 deletion src/v8n.js
View file
Open in desktop
@@ -1,7 +1,9 @@
import Context from "./Context";

function v8n() {
return proxyContext(new Context());
return typeof Proxy !== "undefined"
? proxyContext(new Context())
: proxyContextEs5(new Context());
}

// Custom rules
Expand Down Expand Up @@ -37,6 +39,30 @@ function proxyContext(context) {
});
}

function proxyContextEs5(context) {
const buildRuleSet = ruleSet =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

To avoid side effects this should accept the context as a parameter and return it. This is currently modifying a variable in place and is not very readable.

Also: Does this name make sense? This adds a rule set to the context and not only builds it. Just as a side note.

Object.keys(ruleSet).forEach(prop => {
context[prop] = (...args) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is this variable from? Should be a function argument for buildRuleSet.

const newContext = proxyContextEs5(context._clone());
return newContext._applyRule(ruleSet[prop], prop)(...args);
};
});
sbarfurth marked this conversation as resolved.
Show resolved Hide resolved

buildRuleSet(availableRules);
buildRuleSet(customRules);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This changes context but that's not readable.


Object.keys(availableModifiers).forEach(prop => {
Object.defineProperty(context, prop, {
get: () => {
const newContext = proxyContextEs5(context._clone());
return newContext._applyModifier(availableModifiers[prop], prop);
}
});
});

return context;
sbarfurth marked this conversation as resolved.
Show resolved Hide resolved
}

const availableModifiers = {
not: {
simple: fn => value => !fn(value),
Expand Down