Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Support pulling JavaScript libraries from Asset Packagist #1923

Merged
merged 2 commits into from
Aug 21, 2017
Merged

Support pulling JavaScript libraries from Asset Packagist #1923

merged 2 commits into from
Aug 21, 2017

Conversation

phenaproxima
Copy link
Contributor

Asset Packagist is going to be adopted by Lightning soon as the main way to pull in libraries from Bower and NPM by way of Composer. Since supporting this requires adding a few things to any composer.json which consumes Lightning as a dependency, this PR is needed for DF -- it adds Asset Packagist as a Composer package repository and configures any Bower or NPM libraries to be dropped into docroot/libraries.

Also see acquia/lightning-project#41, acquia/lightning#430, and acquia/df#58.

@grasmash
Copy link
Contributor

I feel a lot of trepidation about this.

I tried asset packagist a while back, like a year ago. I encountered many issues. Some of them were due to the package itself, and some of them arose only when requiring specific packages. Essentially, I found that the dependency resolution just didn't translate very well into non-PHP packages.

For instance, I have found that when using NPM, the exact version of the packages that are installed depends on the version of note that is being run on the host machine. Since composer runs via PHP, it would not properly determine compatible versions of packages.

Maybe things have improved since then.

In general, it just seems strange to me to shoehorn JavaScript dependencies into a PHP dependency manager when there is already a fully featured and reliable JavaScript dependency manager out there (NPM). Are we going to be fighting an uphill battle? Have you considered some way of using NPM?

@phenaproxima
Copy link
Contributor Author

phenaproxima commented Aug 14, 2017

@grasmash, that makes a lot of sense. But I disagree :)

Asset Packagist is a way to bring front-end JavaScript dependencies -- i.e., JS libraries that will be used in a browser only, NOT in Node -- into a project that is primarily server-side PHP. It's meant to overcome the fact that most JavaScript libraries, even ones that are useful only in a browser, do not include a composer.json and are not exposed to Packagist. Asset Packagist isn't aware of NPM or the way it resolves dependencies, and it was never meant to be. If you're writing a Node app in JavaScript, with dependencies listed in package.json, it would be crazy to use Composer over NPM, for the reasons you point out.

This is the use case we're trying to fulfill in Lightning. Lightning is not a Node app, and it never will be. It runs exclusively in a browser, so any JavaScript libraries we're bringing in are meant only for use in the browser and probably wouldn't run on Node anyway -- yet they're listed in NPM and Bower, because those are "the" package repos for anything written in JavaScript. We're therefore trying to take advantage of NPM (and Bower) as package respositories, without caring about their dependency resolution.

Granted, we could use NPM to bring in our JavaScript dependencies. The problem then becomes one of UX and complexity:

  1. As a Lightning user, you MUST have NPM installed -- for no other reason than to install Lightning. This isn't really a problem for our more technical users, but we need to support less technical users as well.

  2. Even if we can assume that NPM is installed, the process of installing Lightning itself becomes more complex. Lightning (the profile) will need to run a post-install script which runs NPM, then moves the installed libraries from node_modules to docroot. But worse yet, any scaffold project that's consuming Lightning (Lightning Project, BLT, DF, and others) will need to run NPM using the package.json provided by Lightning, then run a script to move the installed libraries into the correct place in docroot. And we'd also need a post-update script to do the same work after update, in case any of Lightning's JavaScript dependencies were changed (which calls forth scary scenarios like the Composer update process succeeding, but NPM's failing due to version conflicts, thus leaving the Lightning installation in an uncertain state.) This entire squiggly mess vanishes if we use Asset Packagist.

Not only is this a far cleaner and (for less technical people) easier solution, it's quite harmless. It absolutely does not prevent anyone from managing their JavaScript dependencies exclusively with NPM/Yarn/Bower, if that's what they want to do. It is not a method that anyone should ever use to manage NPM dependencies in a JavaScript application.

So basically, I don't think you should feel too much trepidation about this, although I understand it -- for the limited use case of "I want to bring some browser-only JavaScript libraries into my Composer-based Drupal site", Asset Packagist is a win. For hard-core Node development, it is a non-issue.

@grasmash
Copy link
Contributor

I find your argument convincing.

Are you planning to document the asset packagist strategy, its intentions, and its limitations somewhere?

@grasmash grasmash added the Enhancement A feature or feature request label Aug 14, 2017
@phenaproxima
Copy link
Contributor Author

Are you planning to document the asset packagist strategy, its intentions, and its limitations somewhere?

We absolutely are. @balsama will write a blog post on Lightning's site, and I'm going to write a script that can perform the required modifications to a composer.json that is consuming Lightning, to make it simple for existing users. We did something similar when we switched from webflo's old drupal-composer packagist to the official packages.drupal.org.

I'm quite happy to get both of those things done before this is merged, if you like.

@phenaproxima
Copy link
Contributor Author

Here's the Composer script which can adjust the root composer.json for an already-existing project: https://github.com/acquia/lightning/pull/430/files#diff-891b4fdcd90f4b1da0fa737ec41c6fe3

@phenaproxima
Copy link
Contributor Author

phenaproxima commented Aug 15, 2017

I wrote a blog post to document everything (currently unpublished): http://lightning.acquia.com/blog/round-your-front-end-javascript-libraries-composer

acquia/lightning#431 also includes a Composer script to make it easy for people to add Asset Packagist support to their root composer.json.

@phenaproxima
Copy link
Contributor Author

Discussed with @grasmash, and he pointed out #1931 -- Lightning is making a reasonable, but BLT-incompatible, assumption which breaks this functionality. I will need to add a hook to blt/Updates.php which modifies composer.json in the same way that the Lightning-included update script does.

@grasmash grasmash merged commit 937355a into acquia:8.9.x Aug 21, 2017
grasmash pushed a commit that referenced this pull request Sep 6, 2017
* Added asset packagist.

* Wrote an update hook.
@balsama
Copy link
Contributor

balsama commented Sep 12, 2017

@grasmash Just realized that this hasn't actually made it into a release. Is there an anticipated release date for 8.9.3 (or whatever the first release that includes this will be)?

Lightning committed Bulk Uploads which makes use of this functionality. We're scheduled to release that as early as 20 September. We can either push that back or roll back the Bulk Upload commit if BLT can't release before then, but it'd be a lot cooler if we didn't have to :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Enhancement A feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants