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 the ability to specify a custom directory for assets. #66

Merged
merged 11 commits into from
Jun 7, 2024

Conversation

terabytesoftw
Copy link
Contributor

Q A
Is bugfix?
New feature? ✔️
Breaks BC?

@terabytesoftw terabytesoftw linked an issue May 27, 2024 that may be closed by this pull request
@darvanen
Copy link

Looks good!

I've got a working example for dependent packages at https://github.com/darvanen/drupal-js/tree/foxy-pr-66

To validate:

  • Clone the repo
  • Checkout branch foxy-pr-gg
  • Run composer install

You'll note that the package that gets pulled in to web/modules/collapsiblock needs this PR.

@darvanen
Copy link

darvanen commented May 28, 2024

I've discovered the drupal-js repo doesn't work when you clone the branch directly, for some reason the plugin doesn't run when it's included via a specific package repository.

@terabytesoftw
Copy link
Contributor Author

1.- Clone repository.
2.- Update composer.json.
3.- Composer install.

This is how it works for me, with the cloned repository.

{
    "name": "drupal/recommended-project",
    "description": "Project template for Drupal projects with a relocated document root",
    "type": "project",
    "license": "GPL-2.0-or-later",
    "homepage": "https://www.drupal.org/project/drupal",
    "support": {
        "docs": "https://www.drupal.org/docs/user_guide/en/index.html",
        "chat": "https://www.drupal.org/node/314178"
    },
    "repositories": [
        {
            "type": "github",
            "url": "https://github.com/php-forge/foxy"
        },
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        }
    ],
    "require": {
        "composer/installers": "^2.0",
        "drupal/collapsiblock": "4.x-dev#4d331139",
        "drupal/core-composer-scaffold": "^10.2",
        "drupal/core-recommended": "^10.2",
        "drupal/foxy": "2.x-dev",
        "drush/drush": "^12.5",
        "php-forge/foxy": "0.1.1 as dev-custom-directory"
    },
    "require-dev": {
        "drupal/core-dev": "^10.2"
    },
    "conflict": {
        "drupal/drupal": "*"
    },
    "minimum-stability": "dev",
    "prefer-stable": true,
    "config": {
        "allow-plugins": {
            "composer/installers": true,
            "drupal/core-composer-scaffold": true,
            "phpstan/extension-installer": true,
            "dealerdirect/phpcodesniffer-composer-installer": true,
            "php-http/discovery": true,
            "php-forge/foxy": true
        },
        "foxy": {
            "manager": "npm",
            "root-package-dir": "web/modules/contrib/collapsiblock"
        },
        "sort-packages": true
    },
    "scripts": {
        "post-install-cmd": [
            "./node_modules/vite/bin/vite.js build"
        ],
        "post-update-cmd": [
            "./node_modules/vite/bin/vite.js build"
        ]
    },
    "extra": {
        "drupal-scaffold": {
            "locations": {
                "web-root": "web/"
            }
        },
        "installer-paths": {
            "web/core": [
                "type:drupal-core"
            ],
            "web/libraries/{$name}": [
                "type:drupal-library"
            ],
            "web/modules/contrib/{$name}": [
                "type:drupal-module"
            ],
            "web/profiles/contrib/{$name}": [
                "type:drupal-profile"
            ],
            "web/themes/contrib/{$name}": [
                "type:drupal-theme"
            ],
            "drush/Commands/contrib/{$name}": [
                "type:drupal-drush"
            ],
            "web/modules/custom/{$name}": [
                "type:drupal-custom-module"
            ],
            "web/profiles/custom/{$name}": [
                "type:drupal-custom-profile"
            ],
            "web/themes/custom/{$name}": [
                "type:drupal-custom-theme"
            ]
        }
    }
}

@darvanen
Copy link

I wonder if it's my version of composer, that isn't working for me, it's adding @composer-asset/drupal--foxy but not the same for collapsiblock

@terabytesoftw
Copy link
Contributor Author

I wonder if it's my version of composer, that isn't working for me, it's adding @composer-asset/drupal--foxy but not the same for collapsiblock

Rather than the composer version, you must install the php-forge/foxy branch, and surely @composer-asset/drupal--foxy is installing a stable version, and not the development branch (custom-directory).

@darvanen
Copy link

darvanen commented May 29, 2024

I can create an alpha release of drupal/foxy if it is truly necessary to validate this PR, but it really isn't ready for even alpha yet.

@darvanen
Copy link

I wonder if it has something to do with all of Collapsiblock's dependencies being devDependencies?

@darvanen
Copy link

Ah. The "as" implementations were the wrong way around. You should now be able to:

git clone git@github.com:darvanen/drupal-js.git
cd drupal-js
git checkout foxy-pr-66
composer install

And see the package.json contain "@composer-asset/drupal--collapsiblock": "file:./vendor/foxy/composer-asset/drupal/collapsiblock",

@darvanen
Copy link

darvanen commented Jun 1, 2024

Looks like "config" is a root-only key: https://getcomposer.org/doc/04-schema.md#config

Perhaps we should use "extra" instead?

@terabytesoftw
Copy link
Contributor Author

Looks like "config" is a root-only key: https://getcomposer.org/doc/04-schema.md#config

Perhaps we should use "extra" instead?

Yes i know, but the plugin must handle the scenario that the extension has two behaviors, when it is developed it is rootPackage, and when it is installed it is dependent, in addition all the configuration of Foxy is done in config, and it would be strange and confusing to use extra .

I'm improving the tests, and the PR works fine, in both cases.

Copy link

codecov bot commented Jun 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (cc7b218) to head (d8ce78c).
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##                main       #66   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       319       327    +8     
===========================================
  Files             26        26           
  Lines            737       742    +5     
===========================================
+ Hits             737       742    +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@terabytesoftw terabytesoftw marked this pull request as ready for review June 7, 2024 11:13
@terabytesoftw terabytesoftw merged commit 39b7cad into main Jun 7, 2024
21 checks passed
@terabytesoftw terabytesoftw deleted the custom-directory branch June 7, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Look deeper for package.json?
2 participants