-
Notifications
You must be signed in to change notification settings - Fork 671
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
WIP feature(amd): Allow AMD modules to be decorated by plugins #8085
Conversation
b8fff35
to
b8a075f
Compare
Awesome, can't wait to start debugging js that's gone through multiple levels of modification ;) |
b8a075f
to
08f75b8
Compare
The good thing is (for now) you can see the scripts requested and the file names make it somewhat apparent what's going on. |
Me likes. Thanks @mrclay! |
.. code-block:: php | ||
|
||
// note the view name includes the .js extension, but not .php | ||
elgg_register_external_view('js/my/module.js', true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the true?
This is pretty good. applyDecorations requirement is a little strange -- why can't this just happen in getConfig? |
Still feel uneasy with it, but can't put my finger on exactly why right now. |
I guess it's that I see amd modules in the same vein as java packages or php classes. They are supposed to be static and not modifiable/decoratable at run time. This blends the worlds of di and code loading, and people are already too confused about how that is supposed to work. I'm leaning on my experience with angular here. |
Our requirements seem to be that PHP doesn't output/alter JS, and plugins can do little but add/remove AMD modules. I'm not sure these are realistic as JS dev seems to involve a lot of manually wiring modules together. The mix of AMD and hooks I find yucky even though we have kind of a solution. |
Closed because it was on 1.x, but excitement seems low. |
Includes docs.
To see it working I extended elgg/ckeditor/config in the blog plugin. The JS error CKEditor creates is not caused by this change.
Fixes #8082