If you're not a plugin developer, you can stop reading now.
I have been working on https://github.com/Elgg/Elgg/pull/8526 which removes the js/ and css/ prefixes from all views that have them in favor of using file extension (.js, .css) to designate the content type. For example:
Motivation
I think this would improve the developer experience significantly for all things frontend:
My hope is that more and more of frontend code in Elgg and plugins would transform into self-contained components whose public APIs are well-defined and stable, whose related parts all live close together, and whose names are easily derived from their view locations (and therefore locations easily derived from names, too).
Others seem to generally agree this is a better direction, except backwards compatibility issues give us pause. We need more feedback from developers in the community (that's you!) about whether the benefits are worth the drawbacks for making this change for 2.0.
How I plan to do this in a "mostly-BC" way
Since reorganizing views is so painful (remember Elgg 1.8?), the my PR implements a concept I'm calling "view aliases." View aliases allow views to be referenced and managed under multiple names at a time, though internally to Elgg they are all normalized to one canonical name. This makes it possible to rename views without breaking backwards compatibility (...mostly). All of the following operations Just Work, regardless which name you use to reference the target view:
The drawbacks
That all being said, aliasing is not 100% invisible to plugin developers. There are a couple edge cases where we think issues might arise:
The view_vars, $view_name and view, $view_name hooks will operate on the canonical view name. For example:
elgg_register_plugin_hook_handler('view', 'css/elgg', function($hook, $view_name) { assert($view_name == 'elgg.css') // not "css/elgg" });
Using the view, all hook and examining the view name may not work as intended:
elgg_register_plugin_hook_handler('view', 'all', function($hook, $view_name) { // Won't work because "css/elgg" was aliased to "elgg.css" if ($view_name == 'css/elgg') { // Never executed... } // Won't work because no canonical views start with css/* anymore if (strpos($view_name, 'css/') === 0) { // Never executed... } });
I don't have a good workaround for this, but my thought is that these uses were probably not very robust in the first place...
Relative URLs in CSS may not work correctly if the view generating the CSS is aliased:
/* "css/icon.css" view aliased to "icon.css" */ .icon { /** * Intended target is ".../1234567/default/example.png" * Actually points to ".../1234567/example.png", which is invalid */ background-image: url(../example.png) }
Some workarounds for this issue:
- Always use elgg_get_simplecache_url to generate URLs
- Put resources on the same directory level and make sure they are aliased together
- Elgg core could process views to look for relative URLs in CSS and make sure they are normalized to point to the intended resources
What do you think?
info@elgg.org
Security issues should be reported to security@elgg.org!
©2014 the Elgg Foundation
Elgg is a registered trademark of Thematic Networks.
Cover image by RaĆ¼l Utrera is used under Creative Commons license.
Icons by Flaticon and FontAwesome.
I could imagine a text resource living in js/ so it could be used by the text! RequireJS plugin. E.g. js/component/template.html
If this view name gets normalized to component/template.html.js that's fine as long as it's still served as text/html.
I think that should get normalized to "component/template.html"
I have to admit that I can't fully follow the explanations. I don't fully understand if the view aliases are supposed to be be used mainly for css and js files only (and would have to be declared explicitly to have a canonical name) or if all views have a canonical name by default (also when their location is not changed for Elgg 2.0 compared to an older version of Elgg).
I don't see the advantage of putting everything in a single view directory as opposed to keep the different type of files in different (logicallly structured) directories (css, js, river, widgets, my_plugin_views, admin, etc.). This might result in a fully flat view files structure with many files in a different directory (or maybe even every developer organizing the files differently and you have to find your way through the files from the start with every new plugin).
Regarding the view, $view_name / view, all plugin hook limitations I'm not sure what would be the consequences. Would I simply have to change the view name to check within the callback function to a canonical name or can't I be sure that an alias exist. For example the Fivestar plugin checks for a long list of views using the view, all plugin hook to add the rating html code if a view matches. Is this still possible in any way with the view aliases?
Looking at the canonicalizeViewName() method should help you get what this is about; it's maybe simpler than we're explaining it. We just normalize the view name so there really aren't js/* or css/* views anymore; yet you can still use those file locations and the old view names.
It does mean that if you had both /foo.css and /css/foo.php, those files would now define the same view.
Yes, with the [view, all] hook handler you'd have to rewrite what you matched against. That's easy if you know you need to do it. The concern is how to make sure plugin devs get educated to do that, and there's no way for us to warn site owners that their plugin is not compatible. Things will just silently fail with no indication.
Nothing for plugin authors to do. Core would automatically alias all js/css views to remove the js/css prefix and add a ".js" or ".css" suffix if needed.
You could think about it that way, yea.
To me, there is a difference between names like "js" and "css" (which specify content type of files) and "river", "widgets," "admin" which describe application-level features. The latter are is certainly useful designations. The former not so much, IMO, because they distance related files just because they are a different content type. By "related" I mean the files are likely to change together or they contain some kind of pointers to each other.
If this all doesn't strike you as obviously horrific, then I'd say that's a good sign. The thing I was afraid of was "don't you dare break view hooks like that!"
Maybe the horror is still to come once I come across the BC breaking issues of this change. I have a feeling that I currently just don't fully understand what consequences of this change are.
@Steve I've totally missed your postings before. I'll have to check out the canonicalizeViewName() method. Maybe I understand it better then and possibly I need to see the change "live" to fully understand it even. Too late for that today though. Already deadly tired.
Too tired also for giving any useful advice on this unrelated topic: https://community.elgg.org/discussion/view/2190408/data-directory-in-pub-html. Maybe one of you guys have an idea how to sort out this mess kxx4 brought himself.
@iionly -- Perhaps you'd be so kind as to check out https://github.com/Elgg/Elgg/pull/8526 and test on some of your plugins to see if there's any breakage and if so how extensive? I'd like to avoid as much as possible... I made the change in core without moving all plugin views to the new location and everything seemed fine, but I know third-party plugins can do more exotic things than core plugins at times so a "stress test" would be good I guess.
I've looked into the canonicalizeViewName() method now and I think your explanation is really kind of unclear. While it says more or less that it's only about js and css views I didn't get that and thought the alias thing would be for all views. Maybe this point could be made even more obvious in the docs.
With only the js and css views affected I think that I have no issue with it with the view, all hook usage within my plugins (mainly the Fivestar plugin) as there are no js/css views looked after at least with the default view config provided.
In case of a js view that already has the .js extension it seems there shouldn't be any changes necessary with require(). Right?
- Previous
- 1
- 2
- Next
You must log in to post replies.