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

Allows disabling CSS for a plugin #15881

Closed

Conversation

tylersmalley
Copy link
Contributor

If a plugin does not require styles, a css file will not be produced by the optimizer. Previously this wasn't an issue as all plugins were including duplicated styles (kibana/15816).

This is a blocker before we can re-add #15816

When a plugin is accessed and a CSS file does not exist, we will provide the user an error preventing them from accessing Kibana.

If a plugin does not require styles, a css file will not be produced by the optimizer. Previously this wasn't an issue as all plugins were including duplicated styles (kibana/15816).

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
@tylersmalley tylersmalley force-pushed the disable-css-for-plugin branch from 1554d49 to 149f1f0 Compare January 8, 2018 05:16
@tylersmalley
Copy link
Contributor Author

tylersmalley commented Jan 8, 2018

@spalger @epixa, curious about your thoughts here. Honestly, I am not really excited about the solution. This requires a plugin author to know their build will not produce a css bundle, and disable it. A couple other thoughts are we could set the flag based on if the file exists. Or, we could ignore failures on missing CSS files.

Going to sleep on it and work out some alternative solutions in the morning.

@epixa
Copy link
Contributor

epixa commented Jan 8, 2018

I don't really like this approach, either. It's not that plugins shouldn't need to declare what sort of bundles they have, but I think if we go down that route then it should be wired more concretely into build system rather than just being an informational tool that needs to be kept up to date with the reality that is the actual build output.

We have a UiBundle class now that we use for js files, and that seems to have knowledge about whether there would be js content. Perhaps we can do the same thing for our css bundles, and then we could pass that knowledge onto the jade template? This way as far as the template is concerned, any bundle could potentially not exist, and the logic for whether it does or does not exist resides entirely with a specific bundle object.

I'm not sure how complicated that is, though, and I really think we should get this into 6.2 if we can. A really simple option would be to always create bundles, even if they are empty. This isn't objectively worse than what we do now.

@tylersmalley
Copy link
Contributor Author

@epixa, I agree. I have re-opened the vendor PR with a commit to hopefully address this.

The UiBundle class is responsible for creating the entry files. In a similar fashion, I am ensuring that a style file is created for each entry file. This seems like the easiest approach, for now. In the future, we should not be requesting the CSS file for a bundle unless one was created. With the optimizer, we will need to check to see if there is a style created after the optimizer has completed and update the UiBundle. We could then use this state in the jade template to prevent loading of this asset. After #15880 this will be a very small optimization on only the initial load.

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.

2 participants