Options to prevent emptying `outDir` in dev mode

nlemoine commented 8 months ago

Description

Hello @lhapaipai,

Let me provide some details about the context of my request. I have a custom Twig extension to use svg (icons, etc.) inline:

{{ svg('images/icon.svg`) }}

Prints something like:

<svg viewBox="0 0 32 32">...</svg>

Under the hood, I use the Symfony Asset component. So basically, it resolves a filesystem path instead of resolving an URL. Gets the file content and print it (with a few modification but that's not relevant here).

Works great in production mode. However, in dev mode, vite-symfony-plugin clears everything that's in my build folder, including images, manifest, etc. And I can't get neither the file content or resolve the file path (I could eventually request the file from its URL but that would a shame).

Would you be open to add an option to prevent emptying outDir (or maybe do not empty it at all, can you tell me more details on the reasons that lead you to clear it if I may ask?)

Example

No response

andyexeter commented 8 months ago

Hey @nlemoine,

Isn't this a Vite specific issue rather than this plugin?

Vite empties the outDir by default, but it can be configured in vite.config.js with build.emptyOutDir (see https://vitejs.dev/config/build-options#build-emptyoutdir)

nlemoine commented 8 months ago

No this is triggered by:

https://github.com/lhapaipai/vite-plugin-symfony/blob/3da3614bba98543631d2b0471deaa4221b0e6bcb/src/entrypoints/index.ts#L170

lhapaipai commented 8 months ago

I think you should leave the management of outDir to vite and not place your own files in it.

for your case I advise you to use multiple asset strategies : https://symfony-vite.pentatrion.com/guide/assets.html#multiple-asset-strategies

nlemoine commented 8 months ago

Let me check, but I'm pretty sure the vite option applies to the build mode. I tested locally while modifying vite-plugin-symfony directly in node_modules and you plugin actually clears everything in there except the entrypoints.json file.

nlemoine commented 8 months ago

Yes, this is confirmed, Vite does not clear the whole folder on dev mode. This is caused by the line highlighted above.

nlemoine commented 8 months ago

Do you want a test case to check this?

nlemoine commented 8 months ago

for your case I advise you to use multiple asset strategies

Thanks for the alternative but I don't think it will fit in. Plus, I would have to "build" my images in another build folder which feels a bit odd.

Are you sure this is needed? Clearing the whole build folder is pretty racial and also causes trouble while playing with other plugins. At least making it an option would be nice :)

lhapaipai commented 8 months ago

let's try to understand your use case by deactivating vite-plugin-symfony and imagining a vite simple project

normally each build of vite produces files with different hashes in a dist directory which is in your .gitignore

If you set the build.emptyOutDir option to false and do several builds in a row, you will end up with a build folder full of unnecessary files.

If you specifically place files in the build folder that means they are important files for you, but by default the dist folder is supposed to be in .gitignore?

So I have 2 questions to fully understand your use case.

are you going to put your build folder in your .gitignore and how will you keep track of your unassociated asset files (svg, icons, etc) ?
how will you deal with having all these unnecessary files in your build folder after doing several builds ?

nlemoine commented 8 months ago

are you going to put your build folder in your .gitignore and how will you keep track of your unassociated asset files (svg, icons, etc) ?

I'im gitignoring them but this is irrelevant here.

how will you deal with having all these unnecessary files in your build folder after doing several builds ?

This is my whole point. I do have emptyOutDir enabled and Vite is taking care of clearing the build folder before building for production.

But in dev mode, your great plugin is clearing everything. Not Vite, Vite doesn't clear your build folder in dev mode, this line does: https://github.com/lhapaipai/vite-plugin-symfony/blob/3da3614bba98543631d2b0471deaa4221b0e6bcb/src/entrypoints/index.ts#L170

nlemoine commented 8 months ago

Vite source: https://github.com/vitejs/vite/blob/71dc6a6b7d41c27133f04b92256bead74b8f2127/packages/vite/src/node/build.ts#L459 https://github.com/vitejs/vite/blob/71dc6a6b7d41c27133f04b92256bead74b8f2127/packages/vite/src/node/build.ts#L682

You will see that the emptyOutDir option is only used for production builds :)

lhapaipai commented 8 months ago

actually the option exists for vite and the vite-plugin-symfony does not follow this option so I agree that there is something to change.

indeed there can also be use cases

Vite doesn't clear your build folder in dev mode

I think it's because vite dev server has nothing to do with the outDir, in fact all requests go through http://localhost:5173 it's rollup which generates files in the outDir and it only occurs during the build phase. vite-plugin-symfony generates files in the outDir during both dev and build phases.

you are right we should therefore prevent emptying the build directory. but on the other hand I am not in favor of adding an option to vite-plugin-symfony. I think we should get the value from the vite config: build.emptyOutDir.

what do you think of that ?

Finally, this repository is a read-only sub-tree split of the https://github.com/lhapaipai/symfony-vite-dev. sorry... please make your pull-request on the main repository... thanks

nlemoine commented 8 months ago

I am not in favor of adding an option to vite-plugin-symfony. I think we should get the value from the vite config: build.emptyOutDir. what do you think of that ?

Actually, your plugin should not care, at all, of any clearing (whether in dev or production modes) and respect default Vite behavior (e.g. don't clear/mind of what is in the outDir in dev mode, let Vite clear the outDir if the user configured it with emptyOutDir). Everything else exceeds the plugin scope. If some user ends up with a bunch of unwanted files in its outDir, well, it's his responsability and Vite already has an option for this.

If you're not confortable adding a new option, I would simply remove that line: https://github.com/lhapaipai/vite-plugin-symfony/blob/3da3614bba98543631d2b0471deaa4221b0e6bcb/src/entrypoints/index.ts#L170

That would be the best possible action to take here IMHO. Keeping it, either in its current status or by grabbing the emptyOutDir config, would be wrong for the reasons highlighted above (messing with other plugins, overriding Vite native behavior, etc.)

please make your pull-request on the main repository...

🙈 My bad, I yet knew it. Let me know if you're ok with the approach above, I'll make a PR for this.

lhapaipai commented 8 months ago

what do you think @andyexeter ?

Basically I am more in favor of emptying the build folder when launching the dev server if the build.emptyOutDir option has been defined. why does it bother me so much and why I want to empty it at all costs?

the presence of these files prevents the build_proxy from working in certain cases, preventing requests from being transmitted to the vite dev server and serving stale files

so yes by not emptying the contents of the default folder we can have unexpected behaviors that are difficult to resolve using Vite with Symfony

# config/routes/dev/pentatrion_vite.yaml
_pentatrion_vite:
    prefix: /build
    resource: "@PentatrionViteBundle/Resources/config/routing.yaml"

# vendor/pentatrion/vite-bundle/src/Resources/config/routing.yaml
pentatrion_vite_build_proxy:
    path: /{path}
    defaults:
        _controller: Pentatrion\ViteBundle\Controller\ViteController::proxyBuild
    requirements:
        path: ".+"

when you launch the vite server, vite-plugin-symfony updates its build/.vite/entrypoints.json file, so you lose the link between the generated files and Symfony. the files are therefore unusable for Symfony.

Basically the emptyOutDir option was implemented to prevent deleting the contents of a folder outside the working directory https://github.com/vitejs/vite/issues/1501 https://github.com/vitejs/vite/commit/730d2f0d8081e11c88b1151086d99b23e0c58823

before this commit the behavior was to empty the contents of the folder without warning.

and once again, if Vite don't worry about the build folder in dev mode it is because it does not need it because everything goes through its development server http://localhost:5173 which is not our case with Symfony where pages are served on http://localhost:8000

nlemoine commented 8 months ago

Thanks @lhapaipai for you detailed answer.

the presence of these files prevents the build_proxy from working in certain cases, preventing requests from being transmitted to the vite dev server and serving stale files

Can you provide an example? I don't really see how such case could happen because you already provided classes that ensures that assets/entrypoints are served through Vite dev server.

If I sum up, assets built with Vite (all types of them: css, js, images, etc.) can be served either with:

Both will serve assets from the Vite server (localhost:5173) if I'm right, not the PHP server (localhost:8000). Seems confirmed by running the basic playground example (in which I commented out the the routing part):

Capture d’écran 2024-02-20 à 14 55 59

If some asset is using the PHP server URL, then something is wrong elsewhere, in the way asset are resolved.

when you launch the vite server, vite-plugin-symfony updates its build/.vite/entrypoints.json file, so you lose the link between the generated files and Symfony. the files are therefore unusable for Symfony.

Sorry, I didn't understand that part. Same idea than above, the files are not lost or unsable, they should be served from localhost:5173.

and once again, if Vite don't worry about the build folder in dev mode it is because it does not need it because everything goes through its development server http://localhost:5173 which is not our case with Symfony where pages are served on http://localhost:8000

See screenshot above, in which case an asset could be served through the PHP server?

I also took a look at some similar implementations (vite-plugin-ruby, vite-plugin-laravel) and none of them seems to need clearing the build folder to work properly. I'm not saying that it's wrong, I'm just trying to understand why is it absolutely needed because I'm pretty sure things can be done without this and further more, this is kind of a radical change in Vite default behavior that, if not removed, should probably be optin becauset it may be harmful while using other plugins.

lhapaipai commented 8 months ago

unusable for Symfony

because entrypoints.json rely on http://localhost:5174/ ... entrypoint js files. so a build is needed to have a consistent entrypoints file

for the proxy

Take a look if you disable the enforceServerOriginAfterListening option : https://symfony-vite.pentatrion.com/reference/vite-plugin-symfony.html

lhapaipai commented 8 months ago

in fact I don't understand why the option to empty the build folder when launching the dev server only if the build.emptyOutDir option has been set to true does not suit you. in your case you set emptyOutDir to false and neither vite nor vite-plugin-symfony touches your directory.

nlemoine commented 8 months ago

in fact I don't understand why the option to empty the build folder when launching the dev server only if the build.emptyOutDir option has been set to true does not suit you.

Because like its name states, it's build.emptyOutDir not dev.emptyOutDir and this option is meant to be used on the production build, not the dev build (see https://github.com/lhapaipai/vite-plugin-symfony/issues/40#issuecomment-1952896145).

Using this option in dev mode is a misuse and breaks Vite default behavior IMHO.

lhapaipai commented 8 months ago

+1 for the build prefix

with webpack encore bundle you have one method cleanupOutputBeforeBuild() (from https://github.com/johnagan/clean-webpack-plugin) if you active this option this will empty the build folder when you run encore production and encore dev-server.

I am not closed to an option but I want the default behavior to be the behavior expected by the majority of people. I am aware that you scored one point with the build prefix in build.emptyOutDir

I think that an arbitrator would not be too much in this discussion... 🤯

nlemoine commented 8 months ago

Sorry for being insistent, maybe I haven't been clear enough in my explanations. Anyway, thank you for taking some much time to consider my request 🙏

Your plugin is really great and I love it. Just like you, "I want the default behavior to be the behavior expected by the majority of people" and this is the only point that I truly feel is wrong in it. I'll have a closer look to that case you submitted and will try to come with a solution that I hope will satisfy both of us.

andyexeter commented 8 months ago

IMO, using Vite's build dir for assets outside of Vite's control is non-standard behaviour - this would be true for any build tool i.e. webpack. If my own projects had a requirement to build assets independently of my build tool I wouldn't put them in the same directory as the build tool's generated assets.

With that being said, just because it's non-standard doesn't mean we should hinder developers who want to do it.

Based on the above conversation, I agree that Vite's build.emptyOutDir is not the right option to look at for this. Given that this is an issue related to this package and more generally the use of Vite within Symfony I think a new option should be added to this plugin but the default should be set to true so that it continues working as expected for other developers.

nlemoine commented 8 months ago

Thanks for your feedback @andyexeter :)

IMO, using Vite's build dir for assets outside of Vite's control is non-standard behaviour - this would be true for any build tool i.e. webpack. If my own projects had a requirement to build assets independently of my build tool I wouldn't put them in the same directory as the build tool's generated assets.

Just to clarify, I am not using Vite build folder for storing any other assets than the ones going though Vite. That's not what I'm talking about here. For instance, my images are added in my main entrypoint:

// assets/app.js
import.meta.glob([
    './images/**'
]);

They're hashed, optimized, end up in the manifest.json. Nothing outside my Vite entrypoints is outputted in my build folder. I have emptyOutDir on. In production mode, everything works as expected.

The whole point of this topic is about clearing, automatically, in dev mode, the build folder. That's it 🙂

andyexeter commented 8 months ago

@nlemoine thanks for explaining - this makes sense now.

I actually use the same setup for my projects and those images aren't actually copied to the build folder in dev mode - they are served through a proxy (see https://github.com/lhapaipai/vite-bundle/blob/d007143a488ef2e0f8b0b3d8a1c07ba54da66e11/src/Resources/config/routing.yaml and https://github.com/lhapaipai/vite-bundle/blob/d007143a488ef2e0f8b0b3d8a1c07ba54da66e11/src/Controller/ViteController.php)

I think the issue you're experiencing is possibly because the proxy route isn't working for you. Are you using Docker by any chance?

andyexeter commented 8 months ago

@nlemoine Sorry, I have just re-read your initial comment and can see you're trying to get the filesystem path rather than URL so my theory about the proxy route not working is incorrect.

My first point still stands though, in dev mode the files don't ever actually exist in the build folder.

nlemoine commented 8 months ago

My first point still stands though, in dev mode the files don't ever actually exist in the build folder.

Never stated the opposite :) Does it mean the whole directory has to be wiped off of all existing files in there? I think it shouldn't, because depending on the strategy you chose (you can commit them for example), your production assets might be there and should stay, unless you npm run build again with emptyOutDir: true.

andyexeter commented 8 months ago

But if the files never exist in the build folder in dev mode, how would you access them through your Twig extension?

nlemoine commented 8 months ago

But if the files never exist in the build folder in dev mode, how would you access them through your Twig extension?

Because I have built them before.

andyexeter commented 8 months ago

So you are building your assets with npm run build (a production build) and then wanting to use them in dev whilst running npm run dev?

lhapaipai commented 8 months ago

I realize that a certain number of my answers were not suitable because I did not have the context from the start of the discussion (eg: import.meta.glob) @nlemoine can you give us the detailed steps to follow to reproduce your issue ?

nlemoine commented 7 months ago

Sorry for the late reply. Holidays and too busy until now.

can you give us the detailed steps to follow to reproduce your issue ?

npm run build

My assets are commited.

npm run dev

My assets dispappeared.

I can't file_get_contents('/path/to/icon.svg') (simplified version, actually goes through Vite manifest to get the right hashed path, but that file is deleted, too).

Whether you think it's the wrong way of handling vite assets or not is off topic (it's not actually a choice, more a constraint of the dev env I'm working with).

Two arguments are self sufficients, in my opinion, to remove that emptyDir(build) line (at least as default):

the plugin breaks Vite's default behavior (having files in the build folder in dev mode or not is not a plugin's business), no other backend integration does that
the fact that this commit exists (actually caused by vite-plugin-symfony emptying the build dir, not the plugin using the same hook) should be a hint that something is wrong

Since those arguments were not convincing enough, I'll try to demonstrate the best I can, why this is a (unrelated to my use case) problem.

Let's start with the problem that first lead you to clear that build directory.

because entrypoints.json rely on http://localhost:5174/ ... entrypoint js files. so a build is needed to have a consistent entrypoints file for the proxy

You can write/overwrite that file without clearing the whole directory. No issue here.

the presence of these files prevents the build_proxy from working in certain cases, preventing requests from being transmitted to the vite dev server and serving stale files

This is where the main issue stands.

Let's review the basic playground example and all possible use cases. Note that before runnnig those tests, I disabled the clearing of the build directory and built the assets so the build directory with the same files that would be there on a production env. To make it easier to identify if an asset comes from the build directory, I replaced all colors byd red. Like this:

vite-red

Defaults

Just running the basic example without any changes.

vite-1

👍 All assets are served by vite server.
👍 No assets go through ViteController.
👎 Vite mode is identified as "build" by ViteAssetVersionStrategy (hence the red vite logo). This is an issue I didn't spot until running these tests. In ViteAssetVersionStrategy, you identify the mode based on the presence of a manifest.json file which of course will fail when it's there in dev mode. Relying on viteServer key in entrypoints.jsonseems more reliable to me, so I changed it to:

if (null === $this->viteMode) {
    $this->entrypointsData = $this->fileAccessor->getData($this->configName, FileAccessor::ENTRYPOINTS);
    $this->viteMode = empty($this->entrypointsData['viteServer']) ? 'build' : 'dev';

    $this->manifestData = 'build' === $this->viteMode ? $this->fileAccessor->getData($this->configName, FileAccessor::MANIFEST) : null;
}

👍 All good now, no assets from the build folder is served in dev mode and everything (that has to) goes through vite server.

vite-1 1

`enforceServerOriginAfterListening` set to `false`

This is when assets starts to go through the ViteController.

vite-2

👍 All good now too. Even with built assets existing in the build folder, the dev assets are still served to the browser.

Conclusion

With the little changes I made, I can't find a case where you would serve stale assets, even if they would exist in the build directory. If you have one in mind, I'll be happy to test it.

There's probably an edge case I missed where this could eventually happen. However, like I mentioned before, I find a bit radical to use that emptyDir(build) statement 100% of the time for a 0.x% use case. Further more, that will only happen when you set enforceServerOriginAfterListening to false, which is true by default.

If you're still not convinced, feel free to close that issue, that was my last shot 😄

lhapaipai commented 6 months ago

well you finally convinced me... thank you for that.. I'm slow to relax but ultimately I listened to your arguments and I have nothing to say https://github.com/lhapaipai/symfony-vite-dev/commit/4fc80b8cd443c73adab62507bdb4256cfd7b06a6 fixed in 6.4.4 All the best !

nlemoine commented 6 months ago

Thanks a lot @lhapaipai for taking so much time to friendly discuss that issue and finally consider the changes, it's really appreciated 🙏

And thanks for your awesome package, this is really a great piece of work! 🙌

lhapaipai / vite-plugin-symfony