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.

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

esi:eval support #2

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

esi:eval support #2

wants to merge 7 commits into from

Conversation

indieisaconcept
Copy link
Collaborator

The scope of this change is as follows

Updated tests to use chai & chai-as-promised

Exceptions thrown within tests would result in timeouts. The asserted error would not propagate due to the assertion being thrown within the context of the promise callback. Assertions should now be done as follows.

var esi = ESI( str );
expect(esi).to.eventually.be.eql( 'overidden' ).and.notify(done);

Added support for esi:eval

processESIInclude

This change allows fragments included by esi:eval to modify the parent scope VARS as per the esi spec. This change normalises processESIInclude to processESIFragment to support both include & eval and provides provisions for excluding alt evaluation for eval.

processESITags

The argument signature has changed to support a whitelist of tags to process to support esi:eval. When set tags matching will be excluded from processing.

processESITags(' ... esi string  ... ', ['esi:assign'], 'eval')

travis

Added support for node 0.12 & iojs

Replaced expect.js with chai & chai-as-promised as the only indication of a failing test was a mocha timeout. When a promise throws an exception this is not propogated and so mocha cannot utlitise the error in the context of the test.

Impact: low
This change provides support for esi:eval. Eval supports the overriding of variables defined within the parent scope.

Impact: low
@indieisaconcept
Copy link
Collaborator Author

I realised this almost solves eval. I need to ensure eval is processed before assigns

@MrSwitch
Copy link
Owner

Hey Jon,
That's great, could you explain this statement.

I need to ensure eval is processed before assigns

FYI: Akamai give this example which suggests the 'assign' preceeding an 'eval' needs to be processed.

<esi:assign C_URL="'cust_' + $(HTTP_COOKIE{CustomerCode}) + '.html'"/>
<esi:eval src="$(C_URL)" dca=”none”/>
<esi:vars>
Welcome $(first), how is the weather in $(state)?
</esi:vars>

@indieisaconcept
Copy link
Collaborator Author

Hey Andrew,

I think you can disregard my previous comment. After submitting the PR I re-read the docs and thought I had only partially solved the implementation. I've since read the docs again :-)

The Akamai documentation is quite vague in places, and I assumed I would need to take care of the following.

<esi:assign name="test" value="ok" />
<esi:vars>${test}</esi:vars>
<esi:eval src="override.html" dca=”none”/>
<esi:vars>${test}</esi:vars>

I had a crazy idea I need to process evals before assigns in the parent scope, but after again reading the docs I believe my original assumption ( and implementation in the PR ) is correct.

Every time the ESI processor finds an eval statement in your code, it must stop and
wait for the fragment to be retrieved so that the code can be included in the
namespace. The evals in your code must be processed serially—for two or more evals
in a page, the order can be important.

Cheers

@MrSwitch
Copy link
Owner

Merged in changes for individual spec files.

The shared spec's for esi:include and esi:eval now reside in spec/shared.include.js as recommended by https://github.com/mochajs/mocha/wiki/Shared-Behaviours

Feel free to bounce ideas for the eval runtime processing.

@indieisaconcept
Copy link
Collaborator Author

You've been busy. I like it.

On 16 March 2015 at 21:25, Andrew Dodson notifications@github.com wrote:

Merged in changes for individual spec files.

The shared spec's for esi:include and esi:eval now reside in
spec/shared.include.js
https://github.com/MrSwitch/esi/blob/feature/esi-eval/specs/shared.include.js
as recommended by https://github.com/mochajs/mocha/wiki/Shared-Behaviours

Feel free to bounce ideas for the eval runtime processing.


Reply to this email directly or view it on GitHub
#2 (comment).

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.

2 participants