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

Show warning when manifest is outdated #729

Merged
merged 6 commits into from
Sep 13, 2019

Conversation

barryvdh
Copy link
Contributor

@barryvdh barryvdh commented Sep 9, 2019

This adds a check to the base view to show a warning when the manifest file in the public folder is different from the one in the vendor dir. This prevents broken components or missing updates.

image

The exception is also updated to make it more clear.
Related: #594

@taylorotwell
Copy link
Member

Method name doesn't make sense to me. Make it something like "assetsAreCurrent" or "assetsOutOfDate"

@barryvdh
Copy link
Contributor Author

barryvdh commented Sep 9, 2019

Renamed to assetsAreCurrent

@tomswinkels
Copy link
Contributor

Why not autoupdate the assets?

@barryvdh
Copy link
Contributor Author

The browser usually runs with different permissions and there is no hook in composer to define in this package (only in the root package)

$publishedPath = public_path('vendor/telescope/mix-manifest.json');

if (! File::exists($publishedPath)) {
throw new \RuntimeException('The Telescope assets are not published. Please run: php artisan telescope:publish');
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be a custom exception (like TelescopeAssetsNotPublishedException) that implements ProvidesSolution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically it could yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what isn't clear about this error and the solution though :P

Copy link
Contributor

Choose a reason for hiding this comment

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

There could be a button to run Artisan::call('telescope:publish'), similarly to the migrate button when a table is not found.

@tomswinkels
Copy link
Contributor

@barryvdh can you make a PR for Laravel Horizon?

@barryvdh barryvdh deleted the feat-update-warning branch September 16, 2019 11:01
@barryvdh
Copy link
Contributor Author

I don't think @driesvints was convinced yet.

@tomswinkels
Copy link
Contributor

@barryvdh but i think it is better to do the same in Horizon and Telescope :) @driesvints

@driesvints
Copy link
Member

Sorry, I'm still opposed to do anything about this in the UI. But feel free to make the pr to see if Taylor will accept it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants