Skip to content

[Asset] Ignore missing manifest.json files in non-strict mode #46689

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

Closed
wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix -
License MIT
Doc PR -

Something we missed in #38495 I suppose.

In its current state, the webapp-pack cannot be used without running yarn encore dev or similar: as soon as one uses the {{ asset() }} helper in a Twig file, an exception is thrown because the manifest.json file is missing.

Similar to symfony/recipes#1087, we should instead ignore when this file is missing by default, in non-strict mode only of course.

@GromNaN
Copy link
Member

GromNaN commented Jun 15, 2022

This behavior was not modified by #38495: when the manifest file does not exist, the asset helper always throws an exception (already in 4.4).
If we are to change it, this is a new feature that should target 6.2.

Personally, I enable strict mode on all environments. So yes, the npm run build is required before running the tests or opening a page on dev environment.

What is the point of requiring webpack-encore-bundle in webapp-pack if running the webpack job is optional?

@nicolas-grekas
Copy link
Member Author

The webapp pack experience is plain broken right now, we cannot afford to wait for 6.2 to fix it.
Any alternative idea? Removing webpack encore from the pack?

@nicolas-grekas
Copy link
Member Author

/cc @weaverryan

@GromNaN
Copy link
Member

GromNaN commented Jun 16, 2022

An other alternative would be to create an empty manifest.json file 🙄.

But not installing webpack-encore-bundle by default would make easier to understand why npm/yarn is required.

@weaverryan
Copy link
Member

I think we don't include Encore - so merge symfony/webapp-pack#7 . Running composer require encore is still very easy. And as @GromNaN said, you then know that you will need npm/yarn to run some other commands.

Even if try to include Encore, but without an error, it's not super useful. It would do nothing until you ran the npm/yarn commands. So if you already will need to run some commands, probably composer require encore isn't a big extra command to need to run :).

@nicolas-grekas
Copy link
Member Author

Closing in favor of symfony/webapp-pack#7

@nicolas-grekas nicolas-grekas deleted the asset-not-strict branch June 18, 2022 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants