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

JS modification action #1828

Merged
merged 2 commits into from Feb 14, 2017
Merged

JS modification action #1828

merged 2 commits into from Feb 14, 2017

Conversation

gamma
Copy link
Contributor

@gamma gamma commented Feb 8, 2017

This was part of the PR #1002 - I extracted the JS part to make it available already. I'm working on a new solution to incorporate the suggestions made by @selfthinker for the CSS part.

Add Event to modify the list of javascript files before they are processed. This allows plugins to not have their - or other plugins - script delivered to the client. Creating the list of files upfront might add a little overhead which I think is OK when you can be certain that only JavaScript that really is needed will be delivered to the client.

Multiple Javascript requests, e.g. to only send the jQuery part or just editor/administrator scripts, could be implemented with this modification (I know, jQuery has already been split).

…essed. This allows plugins to not have their - or other plugins - script delivered to the client. Creating the list of files upfront might add a little overhead which I think is OK when you can be certain that only JavaScript that really is needed will be delivered to the client.

Multiple Javascript requests, e.g. to only send the jQuery part or just editor/administrator scripts, could be implemented with this modification (I know, jQuery has already been split).
@mention-bot
Copy link

@gamma, thanks for your PR! By analyzing the history of the files in this pull request, we identified @splitbrain, @selfthinker and @Klap-in to be potential reviewers.

@splitbrain
Copy link
Collaborator

I'm not sure if we need this anymore? Plugins can avoid having their scripts being delivered through js.php by not naming them script.js. Plugins can add additional script calls through the new CONFUTIL_CDN_SELECT event or the traditional TPL_METAHEADER_OUTPUT events. Or, even better, they can load the needed scripts on demand using jQuerys getScript() method.

So the advantage of this event would be to disable some core or foreign plugin files only, right? I'm not sure how useful that is. But I'm happy to be convinced otherwise.

@gamma
Copy link
Contributor Author

gamma commented Feb 8, 2017

Yep, you are correct. At least this is true for plugins that we maintain our selves. What I want to address is (at least my need) to remove javascript from plugins that are definitely not needed for a certain scope, e.g. the view mode for closed wikis. This approach seems to be the most non-intrusive one.

Specifically, I think this is the only way to make the JavaScript more compact and remove not needed parts from our website which runs in a view-only-mode.

An alternative take could be to have plugin authors implement script.js files for different view modes, but I think that would make the system more complicated in many ways.

@splitbrain
Copy link
Collaborator

Can you explain the view only mode you use? How is content edited? Is it synced from somewhere else?

@gamma
Copy link
Contributor Author

gamma commented Feb 8, 2017

We're using a closed wiki as our primary website. Admins and editors can log in if they need to hot-fix something on the website. Edits are primarily done in a staging area, an internal copy of the web server. Changes are synced either using your sync-plugin - which does tremendous work - or by an rsync of the data folder.

Our wiki has over 80 plugins enabled - including the core plugins. Most of them have a little bit of JavaScript (and CSS, but that is going to be another story ;)).

Visitors will only need a subset of these scripts since a lot of them are only convenience for editing. the caching mechanism should make sure that they usually get a static version of the JS. Editors however should receive the full set of JavaScript, probably as an extra roundtrip using TPL_METAHEADER_OUTPUT.

@splitbrain splitbrain added this to the Frusterick Manners milestone Feb 10, 2017
@splitbrain
Copy link
Collaborator

Okay makes sense to me (I even had an idea how this actually might be useful for another plugin). The changes seem minimal, so I'll merge this one soon.

@splitbrain
Copy link
Collaborator

@gamma could you please create the documentation page for the event?

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.

None yet

3 participants