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

(dev/drupal#8) Migrate bower.json into composer.json via composer-downloads-plugin #15044

Merged
merged 3 commits into from
Aug 24, 2019

Conversation

totten
Copy link
Member

@totten totten commented Aug 15, 2019

Before

  • JS and CSS libraries are downloaded via bower.
  • bower is deprecated.
  • bower is slow.
  • bower requires installing NodeJS.
  • bower requires installing Bower.

After

  • composer install downloads the bower_components folder.
  • composer has a better cache.
  • composer is fast.
  • You can setup a dev build with fewer dependencies.
  • All of the bower_components will land in the same canonical location, regardless of project structure and environment.

Comments

This commit is flagged work-in-progress because it depends on an unreleased branch of a composer plugin. I just want to push it up for the moment so some people can see it. I've opened an issue LastCallMedia/ComposerExtraFiles#2 to discuss the path to publication.

This depends on civicrm/composer-downloads-plugin, which is an updated variant of lastcall/composer-extra-files. It has a few key characteristics:

  • It does not require any root-level package configuration or third-party tools/runtimes.
  • It produces a file-structure that closely matches the current one.
  • It produces the same file-structure whether one (a) uses civicrm-core as root-package or (b) uses civicrm-core as a dependency inside a bigger project.
  • It is fairly transparent and tweakable - anyone who wants to submit a PR to add/upgrade/remove JS/CSS dependencies should be able to do so.
  • When we roll out civicrm extension/composer bridge, extensions should be able to use it while being compatible with several different deployment scenarios.

Technical Details

To try this out, I renamed my old bower_components, ran composer, and compared the new/old trees. The substance all looks good -- there are no missing or differing files. However, there is some new detritus. (Edit: This has been resolved by adding support for ignore directives. The subsequent discussion has been revised to match.)

$ mv bower_components{,.orig}
$ composer install
...
$ colordiff -ru bower_components{.orig,} -x .bower.json -x .composer-downloads -x .setupsh.ts

Only in bower_components.orig/checklist-model: test
Only in bower_components.orig/dc-2.1.x: bower.json
Only in bower_components.orig/font-awesome: bower.json
Only in bower_components.orig/jstree: bower.json

These appear to be trivial differences:

  • For checklist-model/test, the old tree simply has an empty folder. There's no real content in either old or new tree.
  • For the bower.json files, the new tree is more consistent with the ignore specification. The difference arises because (in the old tree) bower doesn't let you ignore bower.json. But we don't really need the file at runtime.

See also: https://lab.civicrm.org/dev/drupal/issues/8

@civibot
Copy link

civibot bot commented Aug 15, 2019

(Standard links)

@civibot civibot bot added the master label Aug 15, 2019
composer.json Outdated Show resolved Hide resolved
@totten totten changed the title (WIP) Migrate bower.json into composer.json (WIP) Migrate bower.json into composer.json via composer-extra-files Aug 15, 2019
@totten
Copy link
Member Author

totten commented Aug 16, 2019

I've pushed an update which imports a bunch of the ignore clauses from bower -- the updated description how shows only a couple trivial differences between the old+new trees.

Additionally, to improve readability, I consolidated all the path statements into default path formula.

composer.json Outdated Show resolved Hide resolved
@colemanw
Copy link
Member

@totten I have rebased the latest upstream commits with my select2 repo and tagged it v3.5-civicrm-1.0 per your suggestion.

@totten
Copy link
Member Author

totten commented Aug 16, 2019

Cheers @colemanw. I've pushed an update to use the new tag.

@totten totten force-pushed the master-composer-all-the-things branch from 7f59212 to 6ba8f3d Compare August 17, 2019 01:47
totten added 3 commits August 22, 2019 04:10
This will be replaced in an adjacent commit with updates to composer.json
Before
------

* JS and CSS libraries and downloaded via `bower`.
* `bower` is deprecated.
* `bower` is slow.
* `bower` requires installing NodeJS
* `bower` requries installing Bower

After
-----

* `composer install` downloads the `bower_components` folder
* `composer` has a better cache
* `composer` is fast
* You can setup a dev build with fewer dependencies.

Comments
--------

There are many, many composer plugins which can be referenced when managing
assets. This particular one has an important distinction:

* It does not require root-level package configuration.
* It produces a file-structure that closely matches the current one.
* It works just as well as whether 'civicrm-core' is used as a root-package
  or as an dependency.
@totten totten force-pushed the master-composer-all-the-things branch from 6ba8f3d to 79e677d Compare August 22, 2019 11:16
@totten totten changed the title (WIP) Migrate bower.json into composer.json via composer-extra-files Migrate bower.json into composer.json via composer-downloads-plugin Aug 22, 2019
@colemanw colemanw merged commit 80b3a61 into civicrm:master Aug 24, 2019
@totten totten deleted the master-composer-all-the-things branch August 24, 2019 02:38
totten added a commit to totten/civicrm-core that referenced this pull request Aug 28, 2019
This is a follow-up to civicrm#15044 - since we don't have/need `bower.json`, it
doesn't make sense for `distmaker` to call `bower install`.
@totten totten changed the title Migrate bower.json into composer.json via composer-downloads-plugin (dev/drupal#8) Migrate bower.json into composer.json via composer-downloads-plugin Sep 9, 2019
magnolia61 pushed a commit to magnolia61/civicrm-core that referenced this pull request Oct 13, 2019
This is a follow-up to civicrm#15044 - since we don't have/need `bower.json`, it
doesn't make sense for `distmaker` to call `bower install`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants