Skip to content
This repository was archived by the owner on Feb 4, 2025. It is now read-only.

Update to modern javascript #1

Merged
merged 33 commits into from
Mar 5, 2019
Merged

Conversation

AndyOGo
Copy link

@AndyOGo AndyOGo commented Feb 21, 2019

  • Rewrite to ES6
  • add Babel 7
  • add rollup
  • update QUnit
  • add prettier, eslint, commit hook

@AndyOGo AndyOGo added the enhancement New feature or request label Feb 21, 2019
@AndyOGo AndyOGo self-assigned this Feb 21, 2019
@AndyOGo AndyOGo changed the title refactore: extract operations Update to modern javascript Feb 21, 2019
@AndyOGo AndyOGo force-pushed the refactore/update-to-modern-js branch from d2dadc1 to ee9fa49 Compare February 21, 2019 17:37
@AndyOGo AndyOGo force-pushed the refactore/update-to-modern-js branch from ee9fa49 to e6e627f Compare February 21, 2019 17:57
@AndyOGo AndyOGo force-pushed the refactore/update-to-modern-js branch from bbe1d8a to e68d842 Compare February 21, 2019 18:16
@AndyOGo AndyOGo force-pushed the refactore/update-to-modern-js branch from c5b26ed to 859945b Compare February 22, 2019 12:19
@AndyOGo AndyOGo force-pushed the refactore/update-to-modern-js branch from 04edfe0 to f68c165 Compare February 22, 2019 14:41
@@ -1,25 +1,61 @@
{
"name": "json-logic-js",
"name": "@axa-ch/json-logic-js",
"version": "1.2.2",
Copy link
Author

Choose a reason for hiding this comment

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

I would bump the version to 2.0.0 🤔

@AndyOGo AndyOGo requested review from l-salih and storm1ng February 22, 2019 16:53
@AndyOGo
Copy link
Author

AndyOGo commented Feb 25, 2019

@mjenny
@lina-salih
I searched through the open PRs and found one which rewrote it to ES6 already, just not using modules 🤔
jwadhams#56

return are_missing;
}

missing.withApply = true;
Copy link
Author

@AndyOGo AndyOGo Feb 25, 2019

Choose a reason for hiding this comment

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

@lina-salih
@mjenny
I don't like this hacks of meta data like withApply and code...
The reason I have them is, that some operations need to call apply recursively, and others have names within the API like == which is not a valid JavaScript function or variable name., so:

  • withApply: adds the apply function as a callback as the first argument
  • code: overrides the functions key within the operations object, e.g. equal() function gets overridden by it's code property being ==

This is too complex and error prone 💣

Choose a reason for hiding this comment

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

Do you plan on changing this at some point?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, it sit's on my nerves^^

Choose a reason for hiding this comment

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

Go for it. 💯

Copy link

Choose a reason for hiding this comment

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

Oh.... do you want to change this in the current PR or open an issue? 😖

}

missingSome.code = 'missing_some';
missingSome.withApply = true;
Copy link
Author

@AndyOGo AndyOGo Feb 25, 2019

Choose a reason for hiding this comment

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

@lina-salih @mjenny
Same here code, withApply as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

return data;
}

variable.code = 'var';
Copy link
Author

Choose a reason for hiding this comment

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

@lina-salih @mjenny
Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

var is a reserved keyword and can't be used as function name

}, 0);
}

add.code = '+';
Copy link
Author

Choose a reason for hiding this comment

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

@lina-salih @mjenny
Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

+ is an existing operator

return a / b;
}

divide.code = '/';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

/ is an existing operator

return a % b;
}

modulo.code = '%';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

% is an existing operator

return a - b;
}

substract.code = '-';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

- is an existing operator

return a == b;
}

equal.code = '==';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

== is an existing operator

return !truthy(a);
}

falsy.code = '!';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

! is an existing operator

return a != b;
}

notEqual.code = '!=';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

!= is an existing operator

return a === b;
}

strictEqual.code = '===';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

=== is an existing operator

Copy link

Choose a reason for hiding this comment

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

yes 👍

return a !== b;
}

strictNotEqual.code = '!==';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

!== is an existing operator

@@ -0,0 +1,3 @@
import truthy from '../../helpers/truthy';
truthy.code = '!!';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

! is an existing operator

return b.indexOf(a) !== -1;
}

indexOf.code = 'in';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

in is an existing operator

return a > b;
}

greater.code = '>';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

> is an existing operator

return a >= b;
}

greaterEqual.code = '>=';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

>= is an existing operator

return c === undefined ? a < b : a < b && b < c;
}

lower.code = '<';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

< is an existing operator

return c === undefined ? a <= b : a <= b && b <= c;
}

lowerEqual.code = '<=';
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

<= is an existing operator

return null;
}

condition.code = ['if', '?:'];
Copy link
Author

Choose a reason for hiding this comment

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

Same here code as described in https://github.com/axa-ch/json-logic-js/pull/1/files#r259730014

/if is an existing operator and ?: not a valid variable name and it's and array...

Copy link

@storm1ng storm1ng left a comment

Choose a reason for hiding this comment

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

return are_missing;
}

missing.withApply = true;

Choose a reason for hiding this comment

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

Do you plan on changing this at some point?

@storm1ng
Copy link

Awesome PR, using once again up-to-date components 👍

Copy link

@l-salih l-salih left a comment

Choose a reason for hiding this comment

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

Great job! thank you @AndyOGo 😄

@@ -0,0 +1,12 @@
function add() {
Copy link

Choose a reason for hiding this comment

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

do we need arithmetic operations?

Copy link
Author

Choose a reason for hiding this comment

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

I would kick them

return a === b;
}

strictEqual.code = '===';
Copy link

Choose a reason for hiding this comment

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

yes 👍

};
}

export default createJsonLogic;
Copy link

Choose a reason for hiding this comment

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

awesome

const not_found = b === undefined ? null : b;
let data = this;

if (typeof a === 'undefined' || a === '' || a === null) {
Copy link

Choose a reason for hiding this comment

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

could we export this as a function?

Copy link
Author

Choose a reason for hiding this comment

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

sure as a helper

@AndyOGo AndyOGo merged commit 1fcfd7f into master Mar 5, 2019
@AndyOGo AndyOGo deleted the refactore/update-to-modern-js branch March 5, 2019 11:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants