-
Notifications
You must be signed in to change notification settings - Fork 71
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
Install JS libs with composer #1168
Comments
Thx for pointing this out @nikathone |
Isn't there already a way to install js libraries like npm en yarn? What is the reason for using composer? |
@ibrahimab I'd say the biggest reason for using Composer is that we are already using it to install everything else. It makes more sense to have our primary tool do more than to add additional tools. |
@ibrahimab Drupal 8 uses composer to install modules, so this allows us to define external JS libraries and have them installed along with the PHP module. This is not intended to install only JS libraries. |
I get that, but I don't think this is the way to go. There is a reason npm/yarn exists. They do a lot more than just fetching code and saving to disk. They do a lot of optimization like walking the tree to just include the code that the application uses. I have looked at assets-packagist and it just allows you to download assets into the vendor folder. This also deviates from standards (requiring code from the vendor folder instead of node_modules). Besides, Drupal 8 already has a way of using javascript in modules: documentation This would create extra work, with no definitive benefits and deviates from standards. |
Yeah, but we're not doing front end work here. This is just so that when people install our viewer modules with composer they don't have to manually deploy the library to the /libraries folder. That's it. If we were making a theme or doing a headless frontend I wouldn't use composer either. |
Why wouldn't you use the default drupal 8 method, defining "*.libraries.yml" file(s) then? |
Hi Ibrahim, defining |
@nikathone Isn't OP talking about external projects? And other projects doing it too is not a good enough reason for me. If it was up to me, I would use npm, setup webpacker and use libraries.yml to include the compiled result. |
I think we are not compiling anything right? It's just a way to download a javascript dependency using composer. What is OP? by the way. The reason I gave those projects as an example is that most of the people maintaining them are also drupal core maintainers. Another example is the slick module, in slick.libraries.yml you have an external library declared like
then you can run This doesn't mean that the only way to get the assets locally is to use the composer approach though. In the slick drupal module readme, under requirements, they actually add an option to do it manually.
|
Original Poster
This is why I oppose getting assets through composer, I just think islandora should use standards (npm). Why would we deviate from how everyone else does it because "It makes more sense to have our primary tool do more than to add additional tools. (@bryjbrown)". Using the best tool for the job should be the rule (referring back to the optimizations npm does behind the scenes). |
Is this still something that we want to pursue? I think this isn't a good approach to handling JS dependencies as the above states. |
I also forgot to mention, using npm will significantly reduce downloads, allow developers to override things without going into core modules (like sass variable overrides), css imports css natively, es6 module imports natively, bundling, packaging (even per page/module) and much more. Why not use it? |
I'm in favor of using composer for adding external libraries. I've recently started using it for my projects and it has made site setup a lot easier than attempting manual install, e.g. "repositories": {
"drupal": {
"type": "composer",
"url": "https://packages.drupal.org/8"
},
"asset-packagist": {
"type": "composer",
"url": "https://asset-packagist.org"
}
} ... "installer-paths": {
"drush/Commands/{$name}": [
"type:drupal-drush"
],
"core": [
"type:drupal-core"
],
"modules/contrib/{$name}": [
"type:drupal-module"
],
"modules/custom/{$name}": [
"type:drupal-custom-module"
],
"profiles/contrib/{$name}": [
"type:drupal-profile"
],
"profiles/custom/{$name}": [
"type:drupal-custom-profile"
],
"themes/contrib/{$name}": [
"type:drupal-theme"
],
"themes/custom/{$name}": [
"type:drupal-custom-theme"
],
"libraries/{$name}": [
"type:drupal-library",
"type:bower-asset",
"type:npm-asset"
]
} allowed me to simply |
I think this is a good idea, and is well-supported by Drupal. It would mean we would need to have our own base composer project, since you can't declare repositories in modules' composer files. THis would be a good idea for a few reasons, including a current problem upgrading several moduels at once that I ran into here: Islandora-Devops/islandora-playbook#202 (comment) Currently the ansible playbook manually creates the library directory structure and downloads individual libraries as tasks in the roles/internal/webserver role tasks. These are the steps required for a single library: # masonry library is required by content_browser and not installed by composer due to issue 2971165.
- name: Create drupal library directory.
file:
state: directory
path: "{{ drupal_external_libraries_directory }}"
owner: "{{ webserver_app_user }}"
group: "{{ webserver_app_user }}"
- name: Unarchive masonry library.
unarchive:
src: "https://github.com/desandro/masonry/archive/v3.3.2.zip"
dest: "{{ drupal_external_libraries_directory }}"
creates: "{{ drupal_external_libraries_directory }}/masonry"
remote_src: yes
- name: Rename masonry directory.
command: mv "{{ drupal_external_libraries_directory }}/masonry-3.3.2" "{{ drupal_external_libraries_directory }}/masonry"
args:
creates: "{{ drupal_external_libraries_directory }}/masonry"
This also makes keeping track of what version of Masonry is installed difficult. If Composer could have visibility into libraries it would be easier on a site maintainer. Moving to a different composer.json file I think would warrant its own issue, but I'd be in favour of it. |
Using a composer project would solve a number of problems currently faced by the community.
|
We do this at Born Digital and it works really really well. 👍 from me. |
Not... so sure about this, in the general case? Seems like it would work for some kind of more-or-less turn-key project; however, having the use of asset-packagist be mandatory, baked in the
|
@adam-vessey, as @alxp noted, this would only be for the Isle and Playbook composer files where you really want something to be turn-key. This wouldn't stop institutions from removing the asset-packagist and using a CDN for their own deployments. Modules wouldn't add the asset-packagist packages to their individual composer files, unless they added it to "suggests" as you propose. |
@seth-shaw-unlv: I don't see any constraints on the scope in which this is to be applied in @alxp's comment (or any other comments in the issue)... unless it requires reading between the lines, or happening in comms out-of-band?... which is why I bring it up here, explicitly. Given it's counter-intuitive nature, of the libraries being dependencies of the modules, but having to specify the repositories in the root/project If this is just to be in a Composer project that can be used optionally (and used between both the Playbook and ISLE and wherever desired), without strongly requiring asset-packagist in any of the different modules, then yeah, I'm fine with this. |
Sorry, that was me reading between the lines based on the "you can't declare repositories in modules' composer files" line and the way adding asset-packagist works. We would only be able to add them to the Isle/Playbook composer files. I often see Drupal module descriptions which include notes about either using asset-packagist or installing the libraries via CDN/manually. We should be doing the same for the relevant modules. |
If we're still using Drupal as a guiding force they are moving to |
Add assets-packagist.org to main composer.json repositories for external javascript libraries. Then we can install external js using composer. 🍾 🚀
The text was updated successfully, but these errors were encountered: