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 dependencies in the default package-lock.json file #19298

Closed
Legioth opened this issue May 2, 2024 · 9 comments · Fixed by #19342
Closed

Include Hilla dependencies in the default package-lock.json file #19298

Legioth opened this issue May 2, 2024 · 9 comments · Fixed by #19342

Comments

@Legioth
Copy link
Member

Legioth commented May 2, 2024

Description of the bug

The initially provided package-lock.json file does not include Hilla dependencies. This slows down node_modules population from 8 seconds to 34 seconds on my fast network connection and probably significantly more on slower connections.

Expected behavior

Expected that all dependencies in the default package.json file are also covered by the default package-lock.json file.

Minimal reproducible example

  1. Download a Vaadin 24.4 project with only Flow views from start.vaadin.com
  2. Add a src/main/frontend/views/@index.tsx file with some dummy content
  3. Run mvn.
  4. Shut down after using '...' for frontend package installation is logged but before Frontend dependencies resolved successfully.
  5. Run npm ci and notice errors about missing dependencies.

Versions

  • Vaadin / Flow version: Vaadin 24.4.0.beta1
  • Node version: v18.18.0
  • npm version: 10.5.2
@mcollovati
Copy link
Collaborator

The package-lock.json file is extracted from the (dev|prod)-bundles, that are currently built without elements that trigger Hilla to be considered.

Is the issue only about development mode, or also production build?
If it is only dev-mode, would it be ok to always have Hilla dependencies in package-lock.json, even for a pure Flow application?

I guess that when using the dev-bundle, it could be fine to have also Hilla packages, but I suppose this should not happen with the production bundle for a pure Flow application.

@knoobie
Copy link
Contributor

knoobie commented May 8, 2024

If it is only dev-mode, would it be ok to always have Hilla dependencies in package-lock.json, even for a pure Flow application?

Would it also include them in package.json and enforce their visibility to SBOMs? If so: I don't like :)

@Legioth
Copy link
Member Author

Legioth commented May 8, 2024

Would it also include them in package.json and enforce their visibility to SBOMs?

Those dependencies are currently added on top of the default package.json when it's initially created unless the corresponding Maven dependency has been explicitly excluded. If we change the contents of the default package.json, then those "extra" dependencies would instead be removed from the default if the corresponding Maven dependency is excluded.

@Legioth
Copy link
Member Author

Legioth commented May 8, 2024

We shouldn't add those dependencies to the default prod bundle since that would just increase the size of the bundle without any benefits. I suspect that adding them only to the dev bundle might cause more problems similar to #18979.

@mcollovati
Copy link
Collaborator

So, it looks like we need to have separate pre-compiled dev and prod bundles for Flow / Flow+Hilla, and make Flow unpack the correct resources for the user project, based on the same checks we do to detect if Hilla is used.

@Legioth
Copy link
Member Author

Legioth commented May 8, 2024

Would it be practical to just have an additional package-lock.json rather than also having an additional bundle that would effectively never be used? Or alternatively change the logic so that package-lock.json is never taken from the bundle but we instead always provide a "full" version of package-lock.json and then let npm i prune any unused dependencies from it the first time it's run.

@mcollovati
Copy link
Collaborator

So basically, the current a bundle should only have an additional package-lock.json adapted for Hilla?
I think this is doable, with some maven trick on the platform side, and would require minimal changes on the Flow side.

@Legioth
Copy link
Member Author

Legioth commented May 8, 2024

That might make sense. Just need to verify that there isn't the same kind of slowdown if the package-lock.json file contains redundant entries that we're seeing when entries are missing.

@mcollovati mcollovati self-assigned this May 8, 2024
mcollovati added a commit to vaadin/platform that referenced this issue May 9, 2024
A 'hybrid-package-lock.json', containing also Hilla dependencies, is added to the
vaadin-dev-bundle artifact, so that Flow can chose the appropriate lock file based
on the project configuration.

Part of vaadin/flow#19298
mcollovati added a commit that referenced this issue May 9, 2024
The package-lock.json provided by the pre-compiled dev-bundle contains no Hilla
dependencies, slowing down the first frontend build.
This change attempts to extract a package-lock.json file specific to the hybrid
application from the bundle, possibly falling back on the standard one.

Fixes #19298
manolo pushed a commit to vaadin/platform that referenced this issue May 10, 2024
A 'hybrid-package-lock.json', containing also Hilla dependencies, is added to the
vaadin-dev-bundle artifact, so that Flow can chose the appropriate lock file based
on the project configuration.

Part of vaadin/flow#19298
mcollovati added a commit to vaadin/platform that referenced this issue May 10, 2024
A 'hybrid-package-lock.json', containing also Hilla dependencies, is added to the
vaadin-dev-bundle artifact, so that Flow can chose the appropriate lock file based
on the project configuration.

Part of vaadin/flow#19298
manolo pushed a commit to vaadin/platform that referenced this issue May 10, 2024
A 'hybrid-package-lock.json', containing also Hilla dependencies, is added to the
vaadin-dev-bundle artifact, so that Flow can chose the appropriate lock file based
on the project configuration.

Part of vaadin/flow#19298
mshabarov pushed a commit that referenced this issue May 13, 2024
The package-lock.json provided by the pre-compiled dev-bundle contains no Hilla
dependencies, slowing down the first frontend build.
This change attempts to extract a package-lock.json file specific to the hybrid
application from the bundle, possibly falling back on the standard one.

Fixes #19298
vaadin-bot pushed a commit that referenced this issue May 13, 2024
The package-lock.json provided by the pre-compiled dev-bundle contains no Hilla
dependencies, slowing down the first frontend build.
This change attempts to extract a package-lock.json file specific to the hybrid
application from the bundle, possibly falling back on the standard one.

Fixes #19298
vaadin-bot added a commit that referenced this issue May 13, 2024
…) (#19363)

The package-lock.json provided by the pre-compiled dev-bundle contains no Hilla
dependencies, slowing down the first frontend build.
This change attempts to extract a package-lock.json file specific to the hybrid
application from the bundle, possibly falling back on the standard one.

Fixes #19298

Co-authored-by: Marco Collovati <marco@vaadin.com>
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.4.0.beta3 and is also targeting the upcoming stable 24.4.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment