8000 [AssetMapper] Fixing incorrect exception & adding allowing more realistic error mode by weaverryan · Pull Request #50393 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[AssetMapper] Fixing incorrect exception & adding allowing more realistic error mode #50393

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

Merged
merged 1 commit into from
May 23, 2023

Conversation

weaverryan
Copy link
Member
Q A
Branch? 6.3
Bug fix? yes-ish
New feature? no-ish
Deprecations? no
Tickets None
License MIT
Doc PR Still WIP

Apologies for the late PR:

A) We were accidentally using an exception class from the Asset component. Fixed.

B) We are parsing .js files to find imports. That, by its very nature, won't be perfect, which is fine - if there is an import we don't recognize we can just leave it alone and (ideally) notify the user. Due to the imperfect matching, the strict mode is too strict to be used as a default - it'll cause bug reports. I've fixed this by adding a "warning" mode and using that. It's a balance between an exception (too much) vs not reporting to the user at all (too little).

Thanks!


private function handleMissingImport(string $message, \Throwable $e = null): void
{
switch ($this->missingImportMode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nitpick) match() is preferred by now

@nicolas-grekas
Copy link
Member

Thank you @weaverryan.

@nicolas-grekas nicolas-grekas force-pushed the asset-mapper-fix-exception branch from 068272c to 9f52bf1 Compare May 23, 2023 08:48
@nicolas-grekas nicolas-grekas merged commit 343b6d7 into symfony:6.3 May 23, 2023
@weaverryan weaverryan deleted the asset-mapper-fix-exception branch May 23, 2023 10:12
@fabpot fabpot mentioned this pull request May 27, 2023
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
0