Skip to content

fix filter signature to allow params, issue in test#198

Draft
ilanl wants to merge 2 commits intobgub:masterfrom
ilanl:filter-with-params
Draft

fix filter signature to allow params, issue in test#198
ilanl wants to merge 2 commits intobgub:masterfrom
ilanl:filter-with-params

Conversation

@ilanl
Copy link

@ilanl ilanl commented Aug 9, 2020

@nebrelbug I have an issue with the test I'm adding.
I was expecting to use a filter that receive an array and as obj and returns a promised string based on parameters.
Parameters are received correctly but the obj sent is string instead of array type.
What am I missing?

@ilanl ilanl marked this pull request as draft August 9, 2020 08:15
@bgub
Copy link
Owner

bgub commented Aug 15, 2020

@ilanl this looks perfect!

The problem in the test is that Squirrelly automatically auto-escapes references before passing them through filters (#189). This has the side effect of converting them to strings.

Since this is a fairly common use case, I'm going to do what I discussed in #189 and make the XML-escape filter (e) the last filter applied to a reference, which should solve the problem. I'll ping you once I've done that 😃

@bgub
Copy link
Owner

bgub commented Aug 15, 2020

Hey @ilanl, I just released version 8.0.4 which applies Squirrelly's XML-escape filter after all other filters. Once you merge my recent changes into this PR, the tests should pass perfectly.

@bgub
Copy link
Owner

bgub commented Sep 9, 2020

Hi @ilanl! Another PR was just merged which fixes the filter signature problem. However, I'd love to merge the test from your PR.

Do you mind if I remove the src/containers.ts file from the PR and then re-trigger the Travis CI build?

Thanks!

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