WICKET-7187 type for JavaScript content item, and new JavaScript import map item

[jstuyts]

Added type to JavaScript content header items, and added a JavaScript import map header item.

The first attempt was to make a base class for AbstractJavaScriptReferenceHeaderItem and JavaScriptContentHeaderItem, and move the type to there. But setType(...) in the former returns AbstractJavaScriptReferenceHeaderItem. Moving the type information up the hierarchy would break backward compatibility. The (simple) code for managing the type has simply been copied now.

JavaScriptReferenceType should be called JavaScriptType now, but that would also break backward compatibility. This has been documented.

If the new features are not used, almost everything works as before. The only change to current behavior is that equals(...) and hashCode(...) of JavaScriptContentHeaderItem now also use the type.

[bitstorm]

Hi @jstuyts ,

I also think moving 'type' up int he hierarchy is a good idea, but I would prefer JavaScriptHeaderItem instead of JavaScriptContentHeaderItem as target class. We can do it on master branch where backward compatibility is not required.
For Wicket 10.x branch you solution looks fine, but I wouldn't change JavaScriptContentHeaderItem adding 'type' field. I'd rather keep this class untouched for Wicket 10.x

[jstuyts]

@bitstorm I've had to do some research to understand what is possible with <script>. As usual with HTML, it is quite complex. References can only be classic scripts or modules, while inline content can be classic scripts, modules, import maps, speculation rules and data blocks: https://html.spec.whatwg.org/multipage/scripting.html#attr-script-type

So I think the current implementation of reference header item classes with JavaScriptReferenceType, correctly reflects what is allowed.

What I think is needed are:

• The addition of a attribute type to JavaScriptContentHeaderItem, with its own type hierarchy instead of reusing JavaScriptReferenceType.
• Specialized classes for import maps and speculation rules, to provide a nice API to construct these instead of having to generate JSON yourself with JavaScriptContentHeaderItem. Because of the flexibility of import maps and (probably) speculation rules, these specialized classes may have limitations.

With these changes Wicket will prevent the user from making (some) mistakes, while still allowing full control if needed.

It will look like this. Green types already exists, purple ones are new:

Edit: including PlantUML source for accessibility:

@startuml

class JavaScriptHeaderItem

class JavaScriptContentHeaderItem {
    type
}
JavaScriptHeaderItem <|-- JavaScriptContentHeaderItem

note left of JavaScriptContentHeaderItem
    Gives full control for any inline
    `<script` element, but specialized
    classes for import maps and
    speculation rules are easier to
    work with.
end note

class AbstractJavaScriptReferenceHeaderItem
JavaScriptHeaderItem <|-- AbstractJavaScriptReferenceHeaderItem

class JavaScriptReferenceHeaderItem
AbstractJavaScriptReferenceHeaderItem <|-- JavaScriptReferenceHeaderItem

class JavaScriptUrlReferenceHeaderItem
AbstractJavaScriptReferenceHeaderItem <|-- JavaScriptUrlReferenceHeaderItem

class JavaScriptImportMapHeaderItem << (C,BB33FF) >>
JavaScriptHeaderItem <|-- JavaScriptImportMapHeaderItem

note bottom of JavaScriptImportMapHeaderItem
    Nicer to work with than with
    `JavaScriptContentHeaderItem`
    for import maps. But may have
    limitations.
end note

class JavaScriptSpeculationRulesHeaderItem << (C,BB33FF) >>
JavaScriptHeaderItem <|-- JavaScriptSpeculationRulesHeaderItem

note bottom of JavaScriptSpeculationRulesHeaderItem
    Nicer to work with than with
    `JavaScriptContentHeaderItem`
    for speculation rules. But may
    have limitations.
end note

class JavaScriptReferenceType
AbstractJavaScriptReferenceHeaderItem --> JavaScriptReferenceType

interface JavaScriptContentType << (C,BB33FF) >>
JavaScriptContentHeaderItem --> JavaScriptContentType

enum JavaScriptWellKnownContentType << (E,BB33FF) >> {
    IMPORT_MAP
    JAVASCRIPT
    MODULE
    SPECULATION_RULES
}
JavaScriptContentType <|-- JavaScriptWellKnownContentType

class JavaScriptDataBlockType << (C,BB33FF) >> {
    mimeType
}
JavaScriptContentType <|-- JavaScriptDataBlockType

note bottom of JavaScriptDataBlockType
    Checks `mimeType` is
    not a well-known type.
end note

@enduml

[bitstorm]

@jstuyts hi see the point. The PR looks good to me but as I mentioned in the previous comment I would apply this "as is" to the wicket-10.x branch, while I think we might rework it for branch master where compatibility constraints don't apply. What do you think if we use class JavaScriptHeaderItem as target base class for type field?

[jstuyts]

@bitstorm in my opinion, moving type to JavaScriptHeaderItem is the wrong approach. There are certain types (currently import maps and speculation rules) that can really benefit from a dedicated API. These sub types of JavaScriptHeaderItem would also inherit type, but ignore its value because they already know their type.

With the type hierarchy in the schema above, the API is nicer: the user is prevented from making mistakes. Also, there is no need for breaking compatibility.

The only "downside" is that 2 enum values of JavaScriptReferenceType (TEXT_JAVASCRIPT and MODULE) have to be duplicated in JavaScriptWellKnownContentType. I feel that the improvement of the API is worth it.

What I can do is: make a PR for everything in the hierarchy above, except for JavaScriptImportMapHeaderItem and JavaScriptSpeculationRulesHeaderItem (which I was not going to build as I have found no use for it yet). I can create a separate PR for the import map later. This way, the type for content items will be available sooner.

[jstuyts]

Split the changes into multiple parts:

• Adding a type to JavaScriptContentHeaderItem. (This PR.)
• A dedicated API for import maps. (A future PR. This could take a while.)
• A dedicated API for speculation rules. (I have no need for this now, so will not submit a PR.)

[bitstorm]

@jstuyts the new PR looks good to me! There are just a couple of minor issues to address.

1. Check the URL I've left a note about above
2. Check braces indentation to conform to the code standard (open braces must go on a new line)

Thank you!