Skip to content

Remove dependencies Wicket's modules don't need - #1865

Open
dashorst wants to merge 2 commits into
masterfrom
remove-unneeded-dependencies
Open

dashorst wants to merge 2 commits into
masterfrom
remove-unneeded-dependencies

Conversation

@dashorst

@dashorst dashorst commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Removes the dependencies the dependency audit in #1862 found unused or declared in the wrong module. The two commits are independent and are best reviewed separately.

Use JDK code instead of commons-collections4 where it adds nothing. wicket-request and wicket-tester required collections4 for one call each: CollectionUtils.isEqualCollection in PageParameters.equals and CollectionUtils.isEmpty in WicketTester. Both now use JDK code. PageParameters keeps its order-insensitive comparison by counting occurrences. MarkupContainer keeps LinkedMap, because removing a child from a large container needs its previous sibling in O(1), and no JDK map provides that. collections4 therefore stays on applications' classpath, and nothing changes for them.

Remove dependencies the modules don't use and declare the ones they do. Apart from the docs below, this commit only changes poms and module-info files:

  • wicket-ioc: drop org.ow2.asm:asm (byte-buddy shades its own copy).
  • wicket-tester, wicket-extensions-tester, wicket-native-websocket-tester: declare junit-jupiter-api instead of compile-scoped assertj-core, junit-jupiter-engine and junit-jupiter-params, none of which their code uses.
  • commons-collections4 and commons-fileupload2-jakarta-servlet5 move from wicket-util, which uses neither, to wicket-core, which uses both.
  • jakarta.enterprise:jakarta.enterprise.cdi-api becomes jakarta.cdi:jakarta.cdi-api; 5.0.0 of the former is only a relocation. Jetty still pulls in the old coordinate, so the Jetty websocket server entry excludes it and jakarta.enterprise.lang-model, which would otherwise put a second copy of the CDI API on the classpath.
  • The root pom drops slf4j-ext, which no module uses.
  • The test modules that got AssertJ or JUnit params through wicket-tester declare them in test scope.
  • migration-to-12.md in the repository root gets a Dependencies section with these changes. The README now points to the guide.

What applications see

Described in migration-to-12.md: wicket-tester no longer brings in AssertJ, the JUnit Jupiter engine or JUnit params, and wicket-ioc no longer brings in ASM. A build that got its JUnit engine only through wicket-tester runs no tests after upgrading. The quickstart archetype declares the engine itself.

Testing

mvn clean verify -Pjs-test passes (6613 tests). The first commit also compiles on its own, including its tests. dependency:analyze no longer reports the findings this PR addresses.

Not included, and left for #1862: the EL implementation in wicket-bean-validation, moving jakarta.inject-api out of wicket-ioc, and the root pom's blanket slf4j-api/servlet-api dependencies. The unbuilt testing/ modules are tracked in #1863, and the PageParameters.hashCode inconsistency in #1866.

🤖 Generated with Claude Code

@codecov-commenter

codecov-commenter commented Nov 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.71%. Comparing base (7d4b720) to head (e305d80).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1865      +/-   ##
============================================
- Coverage     62.72%   62.71%   -0.01%     
- Complexity    11512    11516       +4     
============================================
  Files          1254     1254              
  Lines         48665    48668       +3     
  Branches       6845     6845              
============================================
- Hits          30526    30524       -2     
- Misses        15463    15467       +4     
- Partials       2676     2677       +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dashorst
dashorst force-pushed the remove-unneeded-dependencies branch from ff4c75e to 3bee76f Compare November 8, 2026 10:19
dashorst added a commit that referenced this pull request Nov 8, 2026
The first version of this change wrote dir only for right-to-left
languages and left it off otherwise, because left to right is the
browser's default and the attribute would be on every page without
changing how it renders. In review, solomax argued for writing it in
both directions. MDN recommends setting dir explicitly on the root
element even for documents in a single script, and stylesheets that
select on [dir="ltr"] only match when the attribute is there.

WebPage#onHtmlTag now writes dir="ltr" or dir="rtl" next to lang. A dir
in the markup is still kept, and markup that declares its own lang
still gets neither. The expected-output files gain dir="ltr" after
lang.

migration-to-12.md describes the change for applications. The file is
also created by #1865; whichever pull request is merged second keeps
both sections.

GitHub issue #1848

Generated-by: Claude Code (Claude Opus 5.5)
@dashorst

dashorst commented Nov 8, 2026

Copy link
Copy Markdown
Contributor Author

#1870 adds a section to migration-to-12.md as well, with the same header and intro as this PR. Whichever of the two is merged second gets an add/add conflict on that file. It's resolved by keeping both sections.

wicket-request and wicket-tester required commons-collections4 for one
call each: CollectionUtils.isEqualCollection in PageParameters.equals
and CollectionUtils.isEmpty in WicketTester. PageParameters now counts
the occurrences of its named parameters, which keeps the
order-insensitive comparison, and WicketTester checks isEmpty() on the
list getBehaviors returns, which is never null. wicket-request,
wicket-tester and wicket-extensions-tester no longer require the
module, and the examples that used collections4 without declaring it
use JDK classes.

MarkupContainer keeps using LinkedMap: removing a child from a
container with many children needs its previous sibling in constant
time, which no JDK map provides. commons-collections4 therefore stays
on the classpath of every application, so nothing changes for them.

GitHub issue #1862

Generated-by: Claude Code (Claude Opus 5.5)
@dashorst
dashorst force-pushed the remove-unneeded-dependencies branch from 3bee76f to b000632 Compare November 8, 2026 21:43
Comment thread pom.xml
Comment on lines +252 to +253
<groupId>jakarta.cdi</groupId>
<artifactId>jakarta.cdi-api</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching to the new coordinate is right, but with the management entry for jakarta.enterprise:jakarta.enterprise.cdi-api gone, nothing pins the old coordinate any more. Jetty still depends on it: jetty-ee11-websocket-jakarta-server → jetty-ee11-annotations → jetty-ee11-plus → jakarta.enterprise:jakarta.enterprise.cdi-api (unversioned, so Jetty's BOM decides). Before this change the managed 5.0.0 relocated it onto jakarta.cdi:jakarta.cdi-api; now it resolves to 4.1.0 in wicket-examples and wicket-native-websocket-javax. Those modules then carry jakarta.enterprise.* twice, 4.1.0 from Jetty and 5.0.0 under the new coordinate. dependencyConvergence doesn't see it because the coordinates differ.

Could you exclude the old coordinate from the Jetty dependency? The jetty-ee11-websocket-jakarta-server entry in the root <dependencyManagement> covers both modules:

<dependency>
	<groupId>org.eclipse.jetty.ee11.websocket</groupId>
	<artifactId>jetty-ee11-websocket-jakarta-server</artifactId>
	<version>${jetty.version}</version>
	<scope>test</scope>
	<exclusions>
		<exclusion>
			<groupId>jakarta.enterprise</groupId>
			<artifactId>jakarta.enterprise.cdi-api</artifactId>
		</exclusion>
	</exclusions>
</dependency>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in e305d80: dependency:tree showed jakarta.enterprise.cdi-api 4.1.0 in both modules, next to jakarta.cdi:jakarta.cdi-api 5.0.0 in wicket-examples.

Excluding only jakarta.enterprise.cdi-api wasn't enough: jetty-ee11-plus also depends on jakarta.enterprise:jakarta.enterprise.lang-model 4.1.0 directly, and it carries the same jakarta.enterprise.lang.model packages as jakarta.cdi-lang-model-api 5.0.0. The entry now excludes both. After that, wicket-examples has only the jakarta.cdi 5.0.0 artifacts, and wicket-native-websocket-javax has no CDI API on its test classpath. Its Jetty websocket tests still pass, as does the full mvn clean verify -Pjs-test.

@papegaaij papegaaij left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removals check out: I found no remaining use of the dropped dependencies, and the full build and the affected modules' tests pass. One thing to address before merging: the old CDI API coordinate that Jetty still pulls in, see the inline comment on the root pom.

An audit of the module dependencies found several that were declared
but not used, or used but declared in another module:

- wicket-ioc declared org.ow2.asm:asm, left over from the cglib proxies
  (WICKET-6825). byte-buddy ships its own shaded ASM, so nothing used
  it, yet it reached the users of wicket-ioc, wicket-spring and
  wicket-guice.
- wicket-tester and wicket-extensions-tester declared assertj-core,
  junit-jupiter-engine and junit-jupiter-params at compile scope. Their
  code only uses the JUnit Jupiter API, so they now declare
  junit-jupiter-api, as does wicket-native-websocket-tester, which
  declared the engine. assertj-core also brought byte-buddy onto every
  test classpath using WicketTester.
- wicket-util declared commons-collections4 and
  commons-fileupload2-jakarta-servlet5 and required collections4 in its
  module-info, but uses neither. wicket-core, which does, now declares
  both.
- wicket-cdi used jakarta.enterprise:jakarta.enterprise.cdi-api, which
  in 5.0.0 is only a relocation to jakarta.cdi:jakarta.cdi-api. The
  modules now use the new coordinates. Jetty still depends on the old
  coordinate (through jetty-ee11-plus), which the managed 5.0.0 used to
  relocate; it is excluded from jetty-ee11-websocket-jakarta-server,
  together with jakarta.enterprise.lang-model, so wicket-examples and
  wicket-native-websocket-javax don't get the CDI API twice.
- The root pom managed slf4j-ext, which no module uses.

The test modules that got AssertJ or JUnit params through wicket-tester
(wicket-core-tests, wicket-extensions, wicket-auth-roles, wicket-spring,
wicket-examples) declare them in test scope.

Applications that relied on wicket-tester for AssertJ, the JUnit
Jupiter engine or JUnit params, or on wicket-ioc for ASM, have to
declare those themselves. Most test setups already declare a JUnit
engine; without one, no tests run.

migration-to-12.md gets a section on these dependency changes, and the
README points to the guide instead of saying it comes with the release.

GitHub issue #1862

Generated-by: Claude Code (Claude Opus 5.5)
@dashorst
dashorst force-pushed the remove-unneeded-dependencies branch from b000632 to e305d80 Compare November 10, 2026 12:35
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