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

Include hilla-dev-mode to platform #4946

Closed
mshabarov opened this issue Jan 23, 2024 · 8 comments · Fixed by #4968
Closed

Include hilla-dev-mode to platform #4946

mshabarov opened this issue Jan 23, 2024 · 8 comments · Fixed by #4968
Assignees

Comments

@mshabarov
Copy link
Contributor

mshabarov commented Jan 23, 2024

Describe your motivation

Include hilla-dev-mode as a dependency to vaadin-dev.
Remove hilla-dev from Hilla repository as it wouldn't be needed after that.

Additional context

Vaadin/Hilla 24.4.

@tltv
Copy link
Member

tltv commented Jan 26, 2024

Having hilla-dev-mode in vaadin-dev ends up some platform's dev mode integration tests to fail. Logs has just warnings that are not there when hilla-dev-mode is not included.

Maybe it's better to leave hilla-dev-mode out of the vaadin-dev and just add it in hilla repository's hilla pom.xml. Or actually just rename hilla-dev to hilla-dev-mode. (PR)

If hilla-dev-mode has to go to the vaadin-dev, then these warnings need to be dealt some other way.

Server side warning:

[qtp888293905-94] WARN org.atmosphere.websocket.DefaultWebSocketProcessor - Failed invoking AtmosphereFramework.doCometSupport()
java.lang.NullPointerException: Cannot invoke "org.springframework.context.ApplicationContext.getAutowireCapableBeanFactory()" because the return value of "com.vaadin.hilla.ApplicationContextProvider.getApplicationContext()" is null
        at com.vaadin.hilla.devmode.devtools.DevToolsDatabase.initIfNeeded(DevToolsDatabase.java:48)
        at com.vaadin.hilla.devmode.devtools.DevToolsDatabase.handleConnect(DevToolsDatabase.java:29)
        at com.vaadin.base.devserver.DebugWindowConnection.handleConnect(DebugWindowConnection.java:222)
        at com.vaadin.base.devserver.DebugWindowConnection.onConnect(DebugWindowConnection.java:202)
        at com.vaadin.flow.server.communication.PushHandler.lambda$onConnect$3(PushHandler.java:599)
        at java.base/java.util.Optional.ifPresent(Optional.java:178)
        at com.vaadin.flow.server.communication.PushHandler.lambda$onConnect$4(PushHandler.java:598)
        at com.vaadin.flow.server.communication.PushHandler.callWithServiceAndSession(PushHandler.java:227)
        at com.vaadin.flow.server.communication.PushHandler.onConnect(PushHandler.java:595)
        at com.vaadin.flow.server.communication.PushAtmosphereHandler.onConnect(PushAtmosphereHandler.java:103)
        at com.vaadin.flow.server.communication.PushAtmosphereHandler.onRequest(PushAtmosphereHandler.java:77)
        at org.atmosphere.cpr.AsynchronousProcessor.action(AsynchronousProcessor.java:217)
        at org.atmosphere.cpr.AsynchronousProcessor.suspended(AsynchronousProcessor.java:103)
        at org.atmosphere.container.Servlet30CometSupport.service(Servlet30CometSupport.java:67)
        at org.atmosphere.cpr.AtmosphereFramework.doCometSupport(AtmosphereFramework.java:2284)
        at org.atmosphere.websocket.DefaultWebSocketProcessor.dispatch(DefaultWebSocketProcessor.java:574)
        at org.atmosphere.websocket.DefaultWebSocketProcessor.open(DefaultWebSocketProcessor.java:213)
        at org.atmosphere.container.JSR356Endpoint.onOpen(JSR356Endpoint.java:254)
        at org.eclipse.jetty.websocket.jakarta.common.JakartaWebSocketFrameHandler.onOpen(JakartaWebSocketFrameHandler.java:175)
        at org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession.lambda$onOpen$6(WebSocketCoreSession.java:411)
        at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1465)
        at org.eclipse.jetty.server.handler.ContextHandler.handle(ContextHandler.java:1484)
        at org.eclipse.jetty.websocket.core.server.internal.AbstractHandshaker$1.handle(AbstractHandshaker.java:212)
        at org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession.onOpen(WebSocketCoreSession.java:411)
        at org.eclipse.jetty.websocket.core.internal.WebSocketConnection.onOpen(WebSocketConnection.java:542)
        at org.eclipse.jetty.io.AbstractEndPoint.upgrade(AbstractEndPoint.java:451)
        at org.eclipse.jetty.server.HttpConnection.upgrade(HttpConnection.java:419)
        at org.eclipse.jetty.server.HttpConnection.onCompleted(HttpConnection.java:450)
        at org.eclipse.jetty.server.HttpChannel.onCompleted(HttpChannel.java:968)
        at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:485)
        at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:282)
        at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:314)
        at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
        at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.runTask(AdaptiveExecutionStrategy.java:416)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.consumeTask(AdaptiveExecutionStrategy.java:385)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.tryProduce(AdaptiveExecutionStrategy.java:272)
        at org.eclipse.jetty.util.thread.strategy.AdaptiveExecutionStrategy.produce(AdaptiveExecutionStrategy.java:194)
        at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:934)
        at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1078)
        at java.base/java.lang.Thread.run(Thread.java:1623)

Browser console warnings:

[2024-01-26T13:12:30.082Z] [WARNING] http://localhost:8080/prod-mode/VAADIN/@fs/C:/Projects/Vaadin/platform/vaadin-platform-test/node_modules/@vaadin/bundles/vaadin.js?v=e4ab5536 176243:20 "Lit is in dev mode. Not recommended for production! See https://lit.dev/msg/dev-mode for more information."
[2024-01-26T13:12:31.129Z] [WARNING] http://localhost:8080/prod-mode/VAADIN/@fs/C:/Projects/Vaadin/platform/vaadin-platform-test/node_modules/@vaadin/bundles/vaadin.js?v=e4ab5536 176243:20 "Overriding ReactiveElement.createProperty() is deprecated. The override will not be called with standard decorators See https://lit.dev/msg/no-override-create-property for more information."
[2024-01-26T13:12:31.129Z] [WARNING] http://localhost:8080/prod-mode/VAADIN/@fs/C:/Projects/Vaadin/platform/vaadin-platform-test/node_modules/@vaadin/bundles/vaadin.js?v=e4ab5536 176243:20 "Overriding ReactiveElement.getPropertyDescriptor() is deprecated. The override will not be called with standard decorators See https://lit.dev/msg/no-override-get-property-descriptor for more information."
[2024-01-26T13:12:32.652Z] [WARNING] http://localhost:8080/prod-mode/VAADIN/@fs/C:/Projects/Vaadin/platform/vaadin-platform-test/node_modules/@vaadin/bundles/vaadin.js?v=e4ab5536 176243:20 "Element vaadin-side-nav scheduled an update (generally because a property was set) after an update completed, causing a new update to be scheduled. This is inefficient and should be avoided unless the next update can only be scheduled as a side effect of the previous update. See https://lit.dev/msg/change-in-update for more information."
[2024-01-26T13:12:32.654Z] [WARNING] http://localhost:8080/prod-mode/VAADIN/@fs/C:/Projects/Vaadin/platform/vaadin-platform-test/node_modules/@vaadin/bundles/vaadin.js?v=e4ab5536 176243:20 "Element vaadin-side-nav-item scheduled an update (generally because a property was set) after an update completed, causing a new update to be scheduled. This is inefficient and should be avoided unless the next update can only be scheduled as a side effect of the previous update. See https://lit.dev/msg/change-in-update for more information."
[2024-01-26T13:12:32.836Z] [WARNING] http://localhost:8080/prod-mode/ - [DOM] Found 2 elements with non-unique id #button: (More info: https://goo.gl/9p2vKq) %o %o

Those above are extracted from local run, but simlar failures also in CI validation here.

@tltv
Copy link
Member

tltv commented Jan 29, 2024

hilla-dev-mode depends on spring. It adds dependency to spring-boot-autoconfigure so that's probably why tests fails as non-spring ITs fails initializing DevToolsDatabase.

@tltv tltv self-assigned this Jan 29, 2024
@tltv
Copy link
Member

tltv commented Jan 29, 2024

As hilla-dev-mode is already included with hilla when using vaadin-spring-boot-starter, I really don't see point in doing anything more that is already done in hilla PR and platform PR.

@mshabarov
Copy link
Contributor Author

The problem is that when you want to exclude from production build the dependencies only needed for development mode, you usually exclude vaadin-dev.
If you don't add hilla-dev into vaadin-dev, you would need to exclude it explicitly in pom.xml, both vaadin-dev and hilla-dev.
This contradicts to unification goals.

manolo added a commit to vaadin/hilla that referenced this issue Jan 29, 2024
Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
@tltv
Copy link
Member

tltv commented Jan 29, 2024

So it's requirement to have hilla-dev-mode in vaadin-dev so that it's possible to exclude all dev mode dependencies with a single vaadin-dev exclusion for production build. However what complicates the case is that hilla-dev-mode can't be included to non-spring project. no solution yet. Suggestions welcome.

@tltv
Copy link
Member

tltv commented Jan 29, 2024

We could add hilla-dev-mode to vaadin-dev by default. Then hilla apps works ok. Flow Spring apps should also work ok. But Flow non-spring apps are broken. Flow non-spring apps need to exclude hilla-dev-mode somehow to make this work but I don't think it's good idea to force all non-spring apps to exclude hilla-dev-mode explicitly.

To fix the problem above, we could exclude with <exclusions> hilla-dev-mode from vaadin-core's vaadin-dev dependency. Hilla apps would still include hilla-dev-mode. Even when excluding vaadin-dev from production build. Because Hilla apps depend on Spring and they use vaadin-spring-boot-starter. Which adds hilla which adds hilla-dev-mode.
Does that do harm to include hilla-dev-mode in production build in hilla apps? If it's not doing anything bad by default, then this solution could work fine.

manolo added a commit to vaadin/hilla that referenced this issue Jan 29, 2024
chore: change hilla-dev to hilla-dev--mode in hilla

Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
@tltv
Copy link
Member

tltv commented Jan 30, 2024

Making spring dependencies provided in hilla-dev-mode and catching and ignoring NullPointerException from DevToolsDatabase should help to make hilla-dev-mode to work in non-spring projects too.

@Artur-
Copy link
Member

Artur- commented Jan 30, 2024

DevToolsDatabase can be deleted. It is integrated into copilot

tltv added a commit to vaadin/hilla that referenced this issue Jan 30, 2024
Removed DevToolsDatabase and spring-boot-autoconfigure dependency. Feature is integrated into copilot instead.

Related-to: vaadin/platform/issues/4946
tltv added a commit that referenced this issue Jan 30, 2024
ZheSun88 pushed a commit to vaadin/hilla that referenced this issue Feb 1, 2024
* chore: remove DevToolsDatabase from hilla-dev-mode

Removed DevToolsDatabase and spring-boot-autoconfigure dependency. Feature is integrated into copilot instead.

Related-to: vaadin/platform/issues/4946

* chore: remove unused resources from hilla-dev-mode

---------

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
manolo pushed a commit that referenced this issue Feb 2, 2024
manolo pushed a commit that referenced this issue Feb 2, 2024
manolo pushed a commit that referenced this issue Feb 3, 2024
manolo pushed a commit that referenced this issue Feb 5, 2024
manolo pushed a commit that referenced this issue Feb 7, 2024
@github-project-automation github-project-automation bot moved this from ⚒️ In progress to Done in Vaadin Flow ongoing work (Vaadin 10+) Feb 7, 2024
vercel-talented added a commit to vercel-talented/hilla-react that referenced this issue May 4, 2024
Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
vercel-talented added a commit to vercel-talented/hilla-react that referenced this issue May 4, 2024
chore: change hilla-dev to hilla-dev--mode in hilla

Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
vercel-talented added a commit to vercel-talented/hilla-react that referenced this issue May 4, 2024
* chore: remove DevToolsDatabase from hilla-dev-mode

Removed DevToolsDatabase and spring-boot-autoconfigure dependency. Feature is integrated into copilot instead.

Related-to: vaadin/platform/issues/4946

* chore: remove unused resources from hilla-dev-mode

---------

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
byte-dev-hubs added a commit to byte-dev-hubs/hila-java that referenced this issue May 12, 2024
Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
byte-dev-hubs added a commit to byte-dev-hubs/hila-java that referenced this issue May 12, 2024
chore: change hilla-dev to hilla-dev--mode in hilla

Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
byte-dev-hubs added a commit to byte-dev-hubs/hila-java that referenced this issue May 12, 2024
* chore: remove DevToolsDatabase from hilla-dev-mode

Removed DevToolsDatabase and spring-boot-autoconfigure dependency. Feature is integrated into copilot instead.

Related-to: vaadin/platform/issues/4946

* chore: remove unused resources from hilla-dev-mode

---------

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
AceDev24 pushed a commit to AceDev24/hiliaGround that referenced this issue Sep 3, 2024
Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
AceDev24 pushed a commit to AceDev24/hiliaGround that referenced this issue Sep 3, 2024
chore: change hilla-dev to hilla-dev--mode in hilla

Related-to: vaadin/platform/issues/4946

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
AceDev24 pushed a commit to AceDev24/hiliaGround that referenced this issue Sep 3, 2024
* chore: remove DevToolsDatabase from hilla-dev-mode

Removed DevToolsDatabase and spring-boot-autoconfigure dependency. Feature is integrated into copilot instead.

Related-to: vaadin/platform/issues/4946

* chore: remove unused resources from hilla-dev-mode

---------

Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

3 participants