The use of the module sould not be mandatory in production

The module has two primary purposes:
- integration with vite dev server for development,
- and integration with vite asset manifest for production.

The proposed resolution would only work if assets built by vite would have static names, which is by default not the case (they have dynamically generated hashes appended to the filenames).

The module implements vite backend integration as described on https://vitejs.dev/guide/backend-integration.html#backend-integration.

I don't plan to explore the requested use case. That said, I'm open to accept contributions If someone would want to implement it.

Thank you for your nice project and the quick reply.

I'll send you a suggestion, I'd appreciate your feedback and suggestions.

Hello,

This mode of operation also seems more logical to me and allows you to resume the theme without "Vite" if necessary (in the context of reversibility for example).

As the operation differs, it may be interesting to create a new branch (2.x?) in order not to break the sites currently using the 1.x version.

I tested what was done by @Hugo_C → . I change the status of the ticket.

It seems that these changes:
- require manifest.json to be generated for dev mode,
- require vite to be configured to not use hashes in built file names,
- with the module disabled in production (which is the motivation for these changes) it requires to manually set all built chunks/dependencies declared in manifest.json directly in .libraries.yml.

Overall these changes break current functionality of the module, require specific vite configuration and require manual .libraries.yml modifications that currently are done automatically.

I don't see a way to merge this as a default behavior, it would need to be behind some setting/flag that needs to be enabled to switch to proposed behavior. Some documentation that would list caveats/requirements to work in this mode should also be added before possible merge.

Also running phpcs and phpstan with configurations provided in the project repo results in the following new errors:

www-data@172775eb8550:~/drupal/web/modules/contrib/vite$ vendor/bin/phpcs .

FILE: /var/www/drupal/web/modules/contrib/vite/src/Manifest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 128 | ERROR | [x] Expected 1 space after FUNCTION keyword; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: /var/www/drupal/web/modules/contrib/vite/src/Vite.php
----------------------------------------------------------------------------------------------------------------------------
FOUND 13 ERRORS AFFECTING 13 LINES
-----------------------------------------------------------------------------------------------------------------------------
 195 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 203 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 220 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 228 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 251 | ERROR | [ ] Missing short description in doc comment
 252 | ERROR | [ ] Missing parameter comment
 253 | ERROR | [ ] Missing parameter comment
 254 | ERROR | [ ] Missing parameter comment
 255 | ERROR | [ ] Description for the @return value is missing
 257 | ERROR | [x] There must not be a space before the colon in a return type declaration
 258 | ERROR | [x] Opening brace should be on the same line as the declaration
 262 | ERROR | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 267 | ERROR | [x] Expected newline after closing brace
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------

Time: 99ms; Memory: 12MB

www-data@172775eb8550:~/drupal/web/modules/contrib/vite$ vendor/bin/phpstan analyse .
Note: Using configuration file /var/www/drupal/web/modules/contrib/vite/phpstan.neon.
 7/7 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------------------------------------------------------ 
  Line   src/Vite.php                                                                                                
 ------ ------------------------------------------------------------------------------------------------------------ 
  257    Method Drupal\vite\Vite::getNewLibraryPath() never returns null so it can be removed from the return type.  
  267    Else branch is unreachable because previous condition is always true.                                       
 ------ ------------------------------------------------------------------------------------------------------------ 

Hello,

If we follow the Vite Backend integration documentation : https://vitejs.dev/guide/backend-integration.html, it is needed to read the "manifest.json" file in production in order to locate assets in the build folder.

It is not recommended to disable the file hash system because it limits the use of multiple entry files. It is not a way of doing that is tested and documented by the community.

I'm obligated to side with @wotnak on the fact that it is not the rigth approach to disable the vite module in production.

If you are afraid that dev configuration is interfering in production, and have question about security, maybe we can help you understand how to secure things ?

One possible approach to allow not using the module in production while retaining all the advantages of integration with vite manifest.json could be to add an optional build step to `.libraries.yml`. So for development you would do everything the same as now, but when you want to deploy changes to production you would build assets using vite and then execute a build step for `.libraries.yml` that would output rewritten libraries (which is done by default in production) back to the `.libaries.yml` file.

Thank you for your replies.

@aridard
I think there was a misunderstanding regarding the target of the issue:
The idea here is to use the module for developing and building assets for production, but not using Vite to serve those assets at all.
In our case, we're using https://www.drupal.org/project/advagg → to handle the hashing.
The only purpose of Vite for us is to build the assets and work in an advanced front-end environment.

@wotnak
- Require manifest.json to be generated for dev mode.
Either way, if this requirement is not in development, it's in production, but this change could potentially cause disruptions that need to be considered.
- Require Vite to be configured to not use hashes in built file names.
We can probably allow both configurations while parsing libraries, but currently, it's mandatory.
- With the module disabled in production (which is the motivation for these changes), it requires manually setting all built chunks/dependencies declared in manifest.json directly in .libraries.yml.
Yes, and that's not an issue if we don't use Vite to serve them, but there is another point here to consider.

I'm considering trying another approach (with quality check, sorry about that) with a config flag as you suggested.

Hello,

@Hugo'C
If you use this module in a standard way you should not have a nodeJS server runing in production. You should have a manifest.json pointing on already built assets.
The nodeJS server only run in dev mode or during build time.
Vite is not serving assets in production mode, it is your Drupal via the theme libraries. But it is indeed this module who alter the library to serve the right assets based on the manifest.json content.

If you want to alter the way this module is altering assets to meet your need I suggested you use https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21....
If you do not want to serve the library in production, you can do a condition on the environnement and unset the vite library you used in dev mode.

Hello,

The separation sounds better that way but I'm still not comfortable calling this degraded mode "advancedMode" because it sounds like it is a mode for people having a deep understanding of Vite and wanting more control when it is realy a degraded mode not following the documentation and potentially corrupting the dev server run, creating unexpected problems.

I see this mode more like a degraded mode to make impossible out of box interoperabilities possible in a simple way but not the better way.

I'm curious about which https://www.drupal.org/project/advagg → you are using and if some Vite or Rollup plugin already do it ??

Hi,

Thank you all for the time spend on this subject.

As wotnak suggested I guess that the better outcome would be to ignore this request since this module work as intended.

If It's fine for you I close this issue.

wotnak → closed merge request !3