Manage Leaflet library dependency in composer.json

With reference to following link, I have updated composer.json file. Please check attached patch file for this issue.

See: https://www.drupal.org/node/2514612

Thank you,

Committed, thanks for the patch!

Anyone having trouble with that? I can't manage to make this work :(

Problem 1
- Installation request for drupal/leaflet dev-1.x -> satisfiable by drupal/leaflet[dev-1.x].
- drupal/leaflet dev-1.x requires leaflet/leaflet ^0.7.7 -> no matching package found.

@hydra, I was having the same issue when including drupal/leaflet as a dependency of the drupal site. I think it has to do with recursive repositories (https://getcomposer.org/doc/faqs/why-can%27t-composer-load-repositories-...)

I was able to get around this by defining the leaflet repository in my project's root compsoer.json:

{
            "type": "package",
            "package": {
                "name": "leaflet/leaflet",
                "version": "v0.7.7",
                "type": "drupal-library",
                "dist": {
                    "url": "https://github.com/Leaflet/Leaflet/archive/v0.7.7.zip",
                    "type": "zip"
                }
            }
        }

If leaflet would support composer, you wouldn't have to declare the library yourself again in your project's composer.json, however it looks that the Leaflet maintainers do not want to support composer: https://github.com/Leaflet/Leaflet/pull/4409

Thx for the feedback #7!
This should probably be documented in the README?

Hello,

+1 for the documentation into the README.

What about the 1.0.0 version? Project page says it's supported.

MaskOta's patch is nearly there, but fixes the supported versions to 0.7.7 and 1.0.0 specifically. The attached patch allows for 0.7.7 or higher and 1.0.0. or higher and defines a package for the latest version (1.0.1)

In addition to these changes, the leaflet.libraries.yml file will need to point to the script in DRUPAL_ROOT instead of a CDN.

Patch from #15 with the correct library path.

Looks good to me.

The CDN didn't work at all for me, not sure why didn't look into it.

How do you rely on the CDN? If I understand correctly once the dependency has been added to composer.json it's a hard-dependency required for the module to function and those files are the same assets that the CDN is delivering.

That's a good point, you'd either need to install via composer or manually download the library.

Re: #23, a mention in the release notes is probably enough to fix this. I doubt we want to keep a fallback to a CDN forever.

Hi guys, any chance that this will be committed soon? I cannot install your module because of #6, which is dependency for some other modules.

For folks that are using key/value pairs in their repositories section, you can just name the repository "leaflet". Mine looks like this:

...
   "repositories": {
        "drupal": {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        },
        "leaflet": {
            "type": "package",
            "package": {
                "name": "leaflet/leaflet",
                "version": "v0.7.7",
                "type": "drupal-library",
                "dist": {
                    "url": "https://github.com/Leaflet/Leaflet/archive/v0.7.7.zip",
                    "type": "zip"
                }
            }
        }
    }
...

After that, composer require --prefer-dist drupal/leaflet worked fine for me.

Thanks for the patches guys! Unfortunately composer repositories are only supported by the root package so there's not much point adding it to the module unless we instruct people to copy it over. Updating the version to allow 1.0.0 as well would be helpful though otherwise you need some trickery (courtesy of @Sam152 https://www.irccloud.com/pastebin/ilHkBDQ9/)

I get the following when running composer require drupal/leaflet:

Problem 1
    - Installation request for drupal/leaflet ^1.0@beta -> satisfiable by drupal/leaflet[1.0.0-beta1].
    - drupal/leaflet 1.0.0-beta1 requires leaflet/leaflet ^0.7.7 -> no matching package found.

Perhaps there's something missing from my root composer.json?

@cbrody, yes that is what this issue is trying to solve, have a look at #27 for a temporary fix in your root composer file, or the link I posted in #28

Needs work based on #28 (repositories being root package only). I vote for removing the leaflet/leaflet dependency altogether since the library doesn't support composer.

I also vote for removing the leaflet/leaflet dependency and documenting how to add it to the root composer.json file.

Agree that removing the leaflet/leaflet requirement makes sense.

+1
All modules that requires external JS libraries are currently running in the same issue. That's too bad composer doesn't support recursive repositories.
Increasing the priority to Major as a $ composer require drupal/leaflet:~1.0 currently installs the alpha1 (almost a year old) because of this bug.

The attached patch removes the composer requirement and improves the README.txt file to provide instructions.

Huge +1 for #31. As a temporary solution, I removed drupal/leaflet requirement from my root composer and downloaded module into modules/custom folder so that I can continue using composer without breaking leaflet.

#34 misses leaflet.libraries.yml part of #17, therefore needs work.

#34 + #17

It's basically impossible to install this module via composer without this.

Also it seems that leaflet is only available via drmonty/leaflet
https://packagist.org/packages/drmonty/leaflet

This seems an important issue to get right, so after I was asked to apply the patch from #36, I did.
It's in 8.x-1.x-dev now.
I have not tested it myself, so don't shoot the messenger :-)

Rik

Thanks, Rik!

Now we need second beta as well ;) Meanwhile, use composer require drupal/leaflet 1.x-dev instead of composer require drupal/leaflet:~1.0.

Although this got committed, for me there was a problem with the patch in that composer installs leaflet to /libraries/leaflet whereas the leaflet.libraries.yml file has the path as /libraries/Leaflet with a capital 'L'.

This patch corrects it so the libraries load correctly. This patch is based off the current dev.

Anyone able to test this?

You should open another issue to be sure you're going to be seen by the maintainer.

Applied mods to .yml file according to patch by @tanc #41.
By the way... Leaflet JS 1.0.3 is out now ...

Whoops, my bad ;)

OK, I'm new to composer, so sorry I say something stupid. My understanding is that composer is a tool for automatic installation of dependencies. The users expects to run "composer require drupal/leaflet", and that's it. Compose should automatically fetch dependencies on initial install, and automatically update them as needed.

The leaflet module README now has instructions "Add the proper repository to your composer.json file". So there is a manual step, it's not automatic. In the future there is likely to be another manual step to alter the text that was added - e.g. change to version number from 1.0.2.

I think we should aim to remove the need for manual steps. Here is my suggestion

• #8 notes that leaflet library team are reluctant to offer composer. However #37 shows that any other person can make a packagist leaflet wrapper, and there is already drmonty/leaflet.
• Unfortunately we may need to make our own Drupal-specific wrapper "drupal-wrapper/leaflet" to ensure that the library installs to the correct directory (web/libraries/leaflet not vendor/leaflet). We need to add "type": "drupal-library" to the library composer.json (and require "composer/installers"). See docs.
• Edit leaflet module composer.json to add require drmonty/leaflet or drupal-wrapper/leaflet.
• Remove README instruction for manual edit of composer.json.

What does anyone else think? I can open a new issue if that would be clearer. Clearly this issue already has some commits, although arguably it hasn't yet solved the original requested feature.

@AdamPS I completely agree with you, it would be ideal if drupal/leaflet could require a library directly and it would be installed into the correct location. It may be possible using drmonty/leaflet already using the component type although I'm not sure?

In any case it's going to require some config via composer.json to install the type into the right location.

I do think at the very least we should move to using the shim repo (drmonty/leaflet) rather than the main leaflet/leaflet build which contains a lot of stuff we don't need. That would require changing leaflet.libraries.yml as well as the README.

@acbramley Thanks for the reply. As far as I understand it, using the component type in drmonty/leaflet would work perfectly. However it would depend on persuading drmonty to change his/her composer.json. I don't have any idea whether drmonty/leaflet is primarily aimed at Drupal or if there are users from many environments.

Otherwise I guess we would have to create something like "drupal-wrapper/leaflet" so that we can assign the component type. Clearly this is a rather inefficient situation, and it ought to be better, which leads on to....

@MakOta I also would be very keen to see some guidelines and understand the proper way of doing it. If you have any ideas how to achieve this please go ahead! (I am maintainer of a module called RRSSB with the similar situation.)

It's not clear to me that we will get there any time soon. Personally speaking I am in favor of making changes now so that it works as well as possible, and accepting that it probably has to change again if we get a better framework. The alternative - don't make changes until the hoped-for framework is fully developed - seems to leave everyone unnecessarily manually editing composer files in the meantime.

The latest version of 1.0.3 on the download page of the leaflet site provides a link to a zip file containing a streamlined folder structure so Everything is moved up out of dist subfolder. I don't think the shim repository is needed.

Also, separate question, but instead of documenting it in the README.txt, why could we not just include this dependency inside the module's own composer.json file?

@jwilson3 For your "separate question", I believe the answer is that it doesn't work. If you can make it work, please let us know! I think that certain directives are only valid in the top-level composer.json file.

From my point of view, the main objective is to remove the need for manual editing of composer.json. The only way I am aware of to do this is to have a shim repository which contains a composer.json with "type": "drupal-library". The Drupal module can then declare a dependency on the shim repository and the leaflet library will be installed into the place required by Drupal.

Your proposed code changes don't bring us any closer to the "main objective" (to be fair nor do they take us any further away). However the change to the README.txt instructions means that everybody who has already followed them has to edit their composer.json yet again. In my view we should avoid doing that until we have something more likely to be a "final" solution.

To answer the extra question in #50 we can't include the dependency in leaflet's composer.json because the repositories key is root only , that means if i run composer require drupal/leaflet without the repositories key in my root composer.json, I will get a "no matching package found" error for leaflet/leaflet.

Thanks @acbramley for the explanation of root-only repositories. I didnt know about that.

@AdamPS:

Your proposed code changes don't bring us any closer to the "main objective" (to be fair nor do they take us any further away).

While this might be true in the scope of this issue, the fact still stands that Leaflet website has changed how they're packaging and distributing their package, to streamline and reduce the amount of needless extra code we all have sitting around in our sites.

@RdeBoer (or any other maintainer): Given the caveats mentioned by AdamPS, would you be open to accepting the patch as a separate issue to handle the repackaged distro file structure?

@jwilson3 I do see what you are trying to do and why.

However I hope you can see the other side of the argument - I think your patch requires every user of the module to make some manual edits to their main composer.json. Given that this is a certain amount of effort for everyone, maybe it makes sense to use the opportunity to achieve as much as possible. For example, at the very least we could try to stop hard-coding version numbers because that means a manual composer.json edit for every new leaflet library version.

• Dist URL - can you find a link for the latest release?
• License could refer to https://github.com/Leaflet/Leaflet/blob/master/LICENSE
• Maybe it is better to miss out "version:" rather than hard-code one, because the hard-coded one can very easily be wrong in quite a confusing way!

Anyway it's just an idea I will leave it up to you and the maintainers to discuss further.

Dist URL - can you find a link for the latest release?

http://cdn.leafletjs.com/leaflet/master/leaflet.zip, but this is not a good idea to ignore versioning if you want to use semver ;)

Core conversation about the topic: #2873160: Implement core management of 3rd-party FE libraries.

Added Leaflet to title.

Composer will not install the library dependency and fails to install leaflet module. Right now we're manually drag 'n dropping the module into our sites.

Error message:

Problem 1
- Installation request for drupal/leaflet ^1.0@beta -> satisfiable by drupal/leaflet[1.0.0-beta1].
- drupal/leaflet 1.0.0-beta1 requires leaflet/leaflet ^0.7.7 -> no matching package found.

#28 worked for me. Building maps in Drupal 8 using Geofield, Geocoder, Address, Leaflet and Views.

#28 +1

The Beta2 version of the module has been released today, with many new improvements, especially in drupal 8 and php coding standards side.
The Beta2 version of the module embeds in its code base the Leaflet Js library (v. 1.0.3), and doesn't require further actions to the user,
for the Js library injection.
But I created this new Issue: #2942651
that should be a continuation of this one and with a Patch that (applied to the beta2) would implement the management of the Leaflet library dependency in composer.json, instead.
Please move this discussion to it and review that patch solution with your opinions.

@itamair: please pardon me to reopen this issue. Managing the Leaflet library in the composer.json file or in the js directory of the module is a matter of appreciation. Now, the possibility of using Leaflet v1.3.4 seems to fade away as I read the issues about this topic. Would you please have a link or a recommandation to give me in order to run leaflet 8.x-1.0-beta15 with the latest Leaflet library ? Thank you very much.

@gillesbailleux ... until a solid solution will be reached in the now pertinent issue the Leaflet will be embedding the Leaflet Js library at the version 1.0.3.
Everyone should be able to alter this (and point its own versions of the leaflet module) throughout the use of the hook_library_info_alter ...

@itamair: discovering the numerous features of the leaflet_module with Drupal 8 and willing to contribute in the documentation, thank you for your answer.

thanks @gillesbailleux ... you might contribute in the documentation opening a new specific issue and providing a patch on that.
Feel free. Here is Drupal.org community.
Every good willing contributor is really welcome ;-)