jakartaee / jakartaee/faces

Is an inclusion boundary a scope boundary for build time variables?

Open
#2,248 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
127
Forks
59
Avg merge
23h
Merged PRs (30d)
7

Description

Facelets has never said whether a variable in scope where a page includes or invokes something is visible inside the thing being included or invoked. The two implementations answer it in opposite directions, and both can point at the text. Mojarra's answer is the original Facelets one, from the 2006 code which the specification later absorbed. MyFaces' answer comes from the template context its own Facelets implementation gained in 2010.

This is not about tag files leaking their own attributes, which is a defect, tracked in #2247 and eclipse-ee4j/mojarra#3263. This is the underlying model question, and it decides how `c:set`, `ui:param` and the `c:forEach` row variable behave across every inclusion construct in the language.

## The disagreement

Measured on one server, one source tree, over the three constructs which pull in markup from elsewhere. Full method and the wider table in https://github.com/eclipse-ee4j/mojarra/issues/3263#issuecomment-5621745291.

| at the call site | read inside | Mojarra | MyFaces | MyFaces strict |
| --- | --- | --- | --- | --- |
| `` on the page | a plain `ui:include` | `CS` | — | `CS` |
| `` on the page | a tag file | `CS` | — | `CS` |
| `` on the page | a `cc:implementation` | `CS` | — | `CS` |
| `` around it | a plain `ui:include` | `1` | — | — |
| `` around it | a tag file | `1` | — | — |
| `` around it | a `cc:implementation` | `1` | — | — |
| `` on `ui:include` | a plain `ui:include` nested inside it | `UP` | — | `UP` |
| `` on `ui:include` | a tag file inside it | `UP` | — | `UP` |
| `` on `ui:include` | a `cc:implementation` inside it | `UP` | — | `UP` |

The construct makes no difference. Within an implementation all three answer alike, so this is one decision rather than three, and a composite component is not the exception its declared attribute interface might suggest.

The last column is `org.apache.myfaces.STRICT_JSF_2_FACELETS_COMPATIBILITY=true`, which turns MyFaces read-through everywhere except the `c:forEach` row variable. So on the MyFaces side most of the compatibility switch this decision would need already exists, and it has a documented gap of one construct.

Scoped variables are not affected and cross everywhere on both: `c:set` with `scope` of `request`, `view`, `session` or `application`, and the `ui:repeat` row variable, which is documented as request scoped. `#{cc.attrs}` likewise. Only build time variables, which live in the `VariableMapper` and are never stored in a scope, are in question.

## What both implementations already agree on

These are measured the same way, come out identical in all three columns above, and do not move when Mojarra or MyFaces is switched to `Production` or MyFaces to `org.apache.myfaces.CACHE_EL_EXPRESSIONS=noCache`. They bound the question rather than form part of it.

**Nothing set inside an inclusion escapes it.** A `c:set` written inside a `ui:include`, a template, a decoration or a `cc:implementation` is gone once the construct ends. Tag files were the one exception and #2247 closed it.

**Markup written at the call site keeps the call site's variables, wherever it is re-parented to.** Children of a composite component tag which the implementation places with `cc:insertChildren` read the caller's `c:set` and the enclosing `c:forEach` row variable, on every implementation and in every mode, including the one which hides both from the implementation itself. Lexical position beats position in the tree, and the opaque model already honors it.

**An explicitly passed value always arrives.** `x="#{fe}"` written inside a `c:forEach` reads back as the row value through `#{cc.attrs.x}` even where a free `#{fe}` in the same implementation resolves to nothing, because the expression is built where it is written. Whatever is decided cannot break parameter passing. Only free references are in question.

## Both positions are coherent

**Mojarra treats an inclusion as a lexical block.** Names in scope at the call site are visible inside, and what is set inside does not escape.

**MyFaces treats an inclusion as opaque.** Nothing crosses unless it is passed explicitly as `ui:param` or as an attribute, or unless it lives in a scope.

JSP, which Facelets inherits the concept and much of the tag library wording from, does both and picks per construct: `<%@ include file %>` shares the page in both directions, while `` and a JSP tag file share nothing but the scoped attributes. Facelets `ui:include` has a foot in each camp, since it composes at build time into the same view like the static include and takes parameters like the dynamic one. The two parameter mechanisms agree on confinement, `jsp:param` not surviving its `jsp:include` any more than `ui:param` survives its `ui:include`, and differ only in kind, one being a request parameter and the other a variable.

## The two loop variables cannot be unified

The obvious suggestion is to sidestep all of this by making `c:forEach` export its row variable the way `ui:repeat` does, so that both are request scoped and neither is affected by the boundary. That is not available. `c:forEach` applies its body once per iteration at build time, binding `var` in the `VariableMapper` for each pass, and what it leaves behind is N ordinary sibling subtrees with the iteration value already captured in their expressions. There is no per-copy context at render time to hand a request attribute to, which is exactly what `ui:repeat` has as a component and `c:forEach` does not have as a handler.

So the two variables differ in kind whatever is decided here, and the only open question is whether they should also differ in visibility. If the answer is that they should, the `ui:repeat` description needs a sentence, because introducing it as *"an alternative to `h:dataTable` or `c:forEach`"* would then be advice which silently changes behavior for anyone who takes it inside a tag file.

## Arguments on the text, such as it is

The Facelets chapter does not settle it, and where the documentation does compare an inclusion to a JSP construct it does so in both directions. The `ui:` tag library describes `ui:include` as *"very similar to JSP's `jsp:include`"*, which is the construct that shares nothing but the scoped attributes. `FaceletContext.includeFacelet` is documented as *"same as include directive in Jakarta Server Pages"*, which is the construct that shares the page. The second is weaker than it looks, since the parenthetical sits next to a sentence about path resolution and may be about that, but the two sentences are what a reader has, and they point opposite ways.

For the opaque reading: `c:forEach/var` is documented as *"the exported scoped variable ... This scoped variable has nested visibility"*, wording taken from the Jakarta Tags TLD, where the variable is page scoped and page scope does not cross a tag file boundary. And for composite components specifically, `cc:interface` is documented as declaring *"the usage contract"*, which reads as an argument that what crosses is what the contract names.

Against it: only the wording was inherited, not the mechanism. In the original Facelets implementation, `c:set` is headed *"Simplified implementation of c:set"* and declares `var` and `value` as its only attributes, with no `scope` attribute at all and a body which does nothing but `ctx.getVariableMapper().setVariable(varStr, veObj)`. `c:forEach` binds its variable the same way. Scope support and the rejection of `scope="page"` are later additions. So the scopeless form was never page scope by default, and it binds an expression where JSP page scope stores a value. Deriving a boundary rule from the phrase means importing a scope model the implementation never adopted. The `cc:interface` argument has the narrower flaw that a contract governing what a page author may pass in says nothing about what an implementation may read out.

For the read-through reading: `ui:repeat` is introduced as *"an alternative to `h:dataTable` or `c:forEach`"*. An author who takes that at its word and swaps one for the other should not silently lose a variable inside a tag file, and the difference they would be tripping over — whether the tag is a handler or a component — is invisible in the markup.

Against it: the same sentence also names `h:dataTable`, so "alternative" spans two mechanisms and cannot mean equivalence, and the documentation does distinguish the two variables deliberately, one as request scoped and one not.

## What is being asked

A decision, and a sentence stating it, wherever the Facelets chapter comes to describe what an inclusion does. It should be stated in terms of the `VariableMapper` rather than of scope, since importing the word scope is how JSP page scope got into this question in the first place. Either answer leaves one implementation to change, so whichever is chosen should come with a compatibility switch on that side rather than a silent behavior change: pages have relied on read-through since Facelets 1.x, and applications written against MyFaces rely on the isolation.

Whichever way it goes, the other model is worth having as an opt-in, which is what #875 asks for. An attribute naming what it confines, `isolateVariables`, says what is hidden and by saying it also says what is not: a construct with the surrounding variables hidden still reaches request, view, session and application scope, and anyone enabling it as a sandbox would be misreading it. Two details follow from the agreed behavior above. It is caller side on `ui:include` and callee side on the root `ui:composition` of a tag file and on `cc:implementation`, where the author of a reusable unit is the one who knows whether it reads only its own parameters. And it does not apply to `ui:define` bodies or to markup placed by `cc:insertChildren`, which are written at the call site and keep the call site's variables today on every implementation.

#875 is the same question from the opposite direction. It asks for an opt-in attribute on `ui:composition` and `ui:decorate` limiting EL scope to explicitly declared parameters, which is precisely the opaque model requested as a feature, and which MyFaces already ships as its default. If the decision here is opaque, that issue is satisfied by the default; if it is read-through, it becomes the per-construct escape hatch and is worth keeping. Note separately that its stated use case, accepting customer authored templates, is not served either way: scoped attributes cross every boundary in every implementation, so a template with the surrounding EL context hidden still reaches everything through `#{sessionScope.x}` and named beans.

## Related

* eclipse-ee4j/mojarra#3263 — the measurements, and the tag file defects which are deliberately not part of this question
* #875 — opt-in EL scope limiting, open since 2010
* #2247 — specifies the tag file behaviors both implementations agree on, deliberately independent of this question

🤖 Generated with Claude Opus 5

Contributor guide

Open the contributing guide

Research direction

Start by reading the Facelets chapter wording alongside the measurements in eclipse-ee4j/mojarra#3263, and compare the related requests in #875 and #2247. The work is complete when the VariableMapper boundary is decided, the Facelets documentation states it clearly, and the affected implementation has a compatibility switch without changing behavior silently.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
frontend, web-dev
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.