isolate filters object

[lukasapp]

• allows the isolation of the filters object by creating instances of the new AngularExpressions class
• added type definitions, tests and updated the README

[lukasapp]

Also created an issue related to this MR: https://github.com/peerigon/angular-expressions/issues/45

[edi9999]

I think it totally makes sense.

If I'm correct, it was already possible with the API that you showed in #43 right ?

function getFilters(tag: any) {
  return filters[tag];
}
const myExpression = new Parser(new Lexer(), getFilters, { csp: true });

But this brings a less convoluted API to do this, right ?

[lukasapp]

Kind of: the changes from https://github.com/peerigon/angular-expressions/pull/43 were just to fix the type definition for the angular definition of Parser (Here is the line where the code definition is: https://github.com/peerigon/angular-expressions/blob/1fad1ef8f7981b874eacedcc1c94381b50874ee2/lib/parse.js#L4191).

You are correct, that it is currently possible to write some kind of function getFilters() to actually do some sort of isolation. But the code needed to do so is very convoluted and not straight forward. This is why I suggest these changes. Makes it a lot simpler to maintain and see what is going on. Also keeping track of the life time of the instance is a lot more clear this way.

[edi9999]

I was thinking, why not simply allow to pass the filters inside the "expressions.compile" function ?

So instead of :

const expression = new AngularExpressions({
    currency: (input, currency, digits) => {
        const result = input.toFixed(digits);

        if (currency === "EUR") {
            return `${result}€`;
        }

        return `${result}$`;
    },
});

const evaluate = expression.compile(
    "1.2345 | currency:selectedCurrency:2"
);

expect(
    evaluate({
        selectedCurrency: "EUR",
    })
).to.eql("1.23€");

You would simply write :

const evaluate = expression.compile(
    "1.2345 | currency:selectedCurrency:2",
    {
        filters: {
            currency: (input, currency, digits) => {
                const result = input.toFixed(digits);

                if (currency === "EUR") {
                    return `${result}€`;
                }

                return `${result}$`;
            },
        } 
    }
);

expect(
    evaluate({
        selectedCurrency: "EUR",
    })
).to.eql("1.23€");

I think I prefer this approach because it doesn't add the "two-apis" for the same feature.

What do you think ?

[lukasapp]

Yes, that would also make sense in my opinion. But you will have to get rid of the compile.cache object, since this would screw with the output for different tenants using the same filter name. This is why I added a class around the functionality.

But of course this cache object can also be added to the compile functions options and default to an internal object:

const customCache = Object.create(null);
const evaluate = expression.compile(
    "1.2345 | currency:selectedCurrency:2",
    {
        filters: {
            currency: (input, currency, digits) => {
                const result = input.toFixed(digits);

                if (currency === "EUR") {
                    return `${result}€`;
                }

                return `${result}$`;
            },
        } ,
        cache: customCache,
    }
);

expect(
    evaluate({
        selectedCurrency: "EUR",
    })
).to.eql("1.23€");

And the compile function will look something like this:

function compile(src, options) {
  var filters = options.filters || filters;
  var cache = options.cache || cache;

  // ... rest of the code ...
}

Should also still be backwards compatible with the current version this way.

Would this be an acceptable solution?

[edi9999]

What about using the cache only if no filters are given ?

So the code would be :

function compile(src, options) {
  var filters = options.filters || filters;
  var cache = options.filters ? (options.cache || {}) : cache;
  // ... rest of the code ...
}

[lukasapp]

Thats fine for me.

[lukasapp]

I will update the MR according to our notes and get back to you! Thanks for your feedback.

[edi9999]

Sounds great !

[lukasapp]

Hey @edi9999, I have updated the pull request according to our discussed changes in this thread.

Just let me know if there is anything further needed to be done. Thanks!

[edi9999]

Awesome, looks good !

[edi9999]

Released in 1.2.0 !