quarkiverse / quarkus-web-bundler

Create full-stack web apps quickly and easily with this Quarkus extension. It offers zero-configuration bundling for your web app scripts (JS, JSX, TS, TSX), dependencies (jQuery, React, htmx, etc.), and styles (CSS, SCSS, SASS).
Apache License 2.0
16 stars 9 forks source link

[Critical] Web dependencies are packaged in the resulting app #84

Closed ia3andy closed 6 months ago

ia3andy commented 11 months ago

Currently, the web-dependencies (mvnpm or webjars) are included in the packaged app. With the web-bundled they (most of the time) won't be used at runtime.

This is a problem because:

ia3andy commented 11 months ago

@aloubyansky how could we enable extension to do this from Quarkus

ia3andy commented 10 months ago

@aloubyansky any idea on how I could add this as a BuildItem or something in Quarkus core (or any other way), this is really important

aloubyansky commented 10 months ago

Perhaps you could introduce a capability and based on the fact whether it's provided do the necessary?

aloubyansky commented 10 months ago

https://quarkus.io/guides/capabilities

ia3andy commented 10 months ago

I don't see how this can be used to achieve this?

aloubyansky commented 10 months ago

Could you describe in more details what you want to achieve? I am not sure, tbh.

ia3andy commented 10 months ago

@aloubyansky @maxandersen what I could do until we find a solution is to add a route in prod mode on /_static/* to block the other routes. wdyt?

ia3andy commented 10 months ago

This would not fix the size problem but the making public problem (which is the critical one)

maxandersen commented 10 months ago

@ia3andy wether vertx exposed webjars or not is a separate issue here imo?

maxandersen commented 10 months ago

doesn't the exposure only happen when weblocator is added?

ia3andy commented 10 months ago

weblocator

no, it's default, maybe we could add a config for this in core?

ia3andy commented 10 months ago

@maxandersen why is it a separate issue?

webjars and mvnpm are exposed by default which is a big problem in the web-bundler case. I don't see why you would add a webjar or a mvnpm dependency that you don't expose (when not using the web-bundler), so the problem didn't exist before.

maxandersen commented 10 months ago

i'm saying its separate since why does a user have weblocator added when he is using webbundler?

phillip-kruger commented 10 months ago

@maxandersen the webjar locator only change the path to exclude the version. Without it the paths are still served, just with the version in the path.

maxandersen commented 10 months ago

okey but looking in quakrus code base I see only refs to "_static" in devui at the moment and it does not seem to be something webbundler can just block as it would make devui not work?

FroMage commented 10 months ago

So, the rationale here is that you only want to serve the bundles created by web-bundler, and not the resources where it fetched them from (webjar/nvmpm)?

You probably need to produce a build item to exclude jars to participate to the META-INF/resources.

It's funny because at this point, these dependencies essentially become build-time only dependencies, like the -deployment jars.

ia3andy commented 10 months ago

So, the rationale here is that you only want to serve the bundles created by web-bundler, and not the resources where it fetched them from (webjar/nvmpm)?

yes You probably need to produce a build item to exclude jars to participate to the META-INF/resources.

We need this and also exclude them from the output packaged app

It's funny because at this point, these dependencies essentially become build-time only dependencies, like the -deployment jars.

yes

ia3andy commented 10 months ago

okey but looking in quakrus code base I see only refs to "_static" in devui at the moment and it does not seem to be something webbundler can just block as it would make devui not work?

Again my idea is to do it only in prod mode so that wouldn't affect the dev-ui

FroMage commented 10 months ago

Also, I talked with @stuartwdouglas in the past about serving static resources, and I feel we should find a packaging option that doesn't bundle them inside the application.

Whether it's a jar or a native image, these static resources can be bigger than the executable code. HTML, JS, CSS, images, SVG, PDFs… these should be ideally outside of the jar or executable, and served via the more efficient sendfile syscall, instead of being served from memory or who knows where. I think Vert.x extracts static resources into a temp folder and serves them from there, but I'm not entirely sure.

maxandersen commented 10 months ago

@FroMage totally agree we should have that option but doesn't change we need to be able to exclude the jars from the distro when there is a "bundle" that is there already.

that bundle or webjars etc would be nice to be able to have externally to the jar - but I see that as separate concern?

ia3andy commented 10 months ago

Ok I've fixed the main issue: https://github.com/quarkiverse/quarkus-web-bundler/commit/ab911e8f8f740957b21a27893b78436fabf2cd86

The fix is a workaround pending a proper solution in vertx-web (either a BuildItem or a config): https://github.com/quarkusio/quarkus/issues/36409

I will release tomorrow.

ia3andy commented 10 months ago

The issue is now to exclude the web deps jars from the packaged app for size optimization.

phillip-kruger commented 10 months ago

@FroMage @maxandersen yes exactly right, so that is what I discussed with @aloubyansky . We either need a way for an extension to exclude certain jars from the final artifacts, or we need to be able to mark these dependencies as provided/compile-only and still get a hold of them in an extension so that we can bundle, and let maven/gradle take care of the exclusion in the final product.

I prefer option one, as that means the user can still mark the dependencies as compile/runtime, as the resources is available at runtime (just bundled). With option 2 the user might think that they will provide the implementation at runtime.

@ia3andy Dev-ui will work, even if we exclude the jars or block static. It loads the jars directly from your m2 folder (not bundled with the app) and use a custom vertx router that serve under /q

ia3andy commented 10 months ago

Thanks @phillip-kruger it's good to know I'll add the BuildStep on dev too then

ia3andy commented 10 months ago

If we can go for solution 1, then I think it would be cool to have:

This "kinda" make sense and would allow the user to configure exactly what they want to do (if they want one or two specific deps to be accessible from the index.html for some reason)

ia3andy commented 9 months ago

@aloubyansky could we implement solution 1 "A way for an extension to exclude certain jars from the final artifacts"? This way from web-bundler I could do what I suggested above: compile = bundled but not served (default), excluded from the extension runtime = bundled + served

AFAIK we can already know if a jar is compile or runtime from an extension.

aloubyansky commented 9 months ago

I'd be careful enabling an easy way to exclude classpath elements from a production package using build items. The feature wouldn't be exclusive for the web bundler, it'll be available for the whole ecosystem. An extension would have to make sure it is an exclusive user of everything a classpath element (a jar, a dir, etc) contains before removing it. Then perhaps the dependencies of that classpath element should also be considered for removal (unless there are other consumers of those), because if don't there could be "orphaned" not used classpath elements after removing some nodes.

The scope (or Gradle config) based approach would make a more coherent config. Essentially, that's the purpose of build time dependencies - they contribute to the build in some way but then aren't present at runtime.

However, both Maven and Gradle resolvers will have to be fixed to allow us have a classpath consistent with the underlying resolver impl.

I'm considering the following bad idea as a workaround:

  1. allow using wildcards when configuring exclusions of artifacts from classpath (and the packaged app) https://quarkus.io/guides/class-loading-reference#quarkus-class-loading-configuration-class-loading-config_quarkus.class-loading.removed-artifacts
  2. expose removed artifacts through the ApplicationModel using a dedicated dependency flag

The reason this seems to be a lesser evil to me is that what is configured for removal will not be discoverable on the build classpath but you could still consult the removed dependencies knowing they won't be included in the final package if you like.

I suppose the web bundler extension would configure org.mvnpm and possibly other dedicated to web-bundling resources groupIds for removal in its extension metadata. Would that work?

ia3andy commented 8 months ago

Until we have an integrated solution and for previous version of Quarkus, here is the workaround with Maven for packaging jar (this could be changed to filter out webjars):

<plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-antrun-plugin</artifactId>
                <version>${maven-antrun-plugin.version}</version>
                <executions>
                    <execution>
                        <id>delete-mvnpm-deps</id>
                        <phase>package</phase>
                        <goals>
                            <goal>run</goal>
                        </goals>
                        <configuration>
                            <target>
                                <!-- Delete libraries starting with org.mvnpm -->
                                <delete includeemptydirs="true">
                                    <fileset dir="${project.build.directory}/quarkus-app/lib/main">
                                        <include name="org.mvnpm*"/>
                                    </fileset>
                                </delete>
                            </target>
                        </configuration>
                    </execution>
                </executions>
            </plugin>

For other packaging type or gradle, we could find a similar workaround.