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

Add dropzonejs composer merge #173

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from

Conversation

andybroomfield
Copy link
Contributor

See localgovdrupal/localgov#729

What does this change?

Adds the wikimedia composer merge plugin, and then uses that to load dropzonejs composer.libraries.json to require the dropzone JS library.

How to test

Composer install or composer update --lock should install the dropzonejs libary once that module is installed.

How can we measure success?

Module gets installed

Have we considered potential risks?

Others may want to have their own dropzone library, so will have to remove this.
Those who have created localgov drupal projects already will need to copy this step.

Images

n/a

Accessibility

n/a

@finnlewis finnlewis marked this pull request as ready for review December 10, 2024 12:08
@finnlewis
Copy link
Member

Discussing in Merge Tuesday:

@stephen-cox leaning towards composer-merge plugin rather than the npm asset packagist

I suggest we test this again, get the tests running, then merge this to get things moving on localgovdrupal/localgov#729

Note: we'll need to add notes to the README.md and release notes to clarify this appraoch.

@andybroomfield andybroomfield force-pushed the feature/3.x-composer-merge branch from 0f43587 to 650f374 Compare December 10, 2024 12:33
@andybroomfield
Copy link
Contributor Author

andybroomfield commented Feb 5, 2025

This method does work, with a couple of caveats.
The first time the requirement for composer require drupal/simple_media_bulk_upload is run, this won't download the library as composer won't be aware of the merged libraries yet. Running composer update drupal/simple_media_bulk_upload -W (or dropzonejs) is required to get it pick up the merged plugins and download.
See localgovdrupal/localgov_assets#2

Also uninstalling / replacing does require a bit of composer.json surgery to remove it first before adding otherwise a enyo/dropzone can not be found error occurs.

@finnlewis
Copy link
Member

I think we should go with this!

Existing sites will have someone managing the composer.json and can make the call on how to include libraries, following release notes in some cases.

For new installs, they will get a good starting point.

@finnlewis
Copy link
Member

@andybroomfield are we going with it like this?

Shall we just merge it?

@finnlewis finnlewis self-requested a review February 11, 2025 12:37
@finnlewis
Copy link
Member

@willguv this is the one we're pushing for! With appropriate release notes.

@finnlewis finnlewis self-assigned this Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress - Everyone else
Development

Successfully merging this pull request may close these issues.

2 participants