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

Look deeper for package.json? #63

Closed
darvanen opened this issue May 20, 2024 · 17 comments · Fixed by #66
Closed

Look deeper for package.json? #63

darvanen opened this issue May 20, 2024 · 17 comments · Fixed by #66
Labels
Estado: mejora Nueva función o petición

Comments

@darvanen
Copy link

Not all php libraries keep their JS dependencies at the root of the library, instead opting to have them contained in a subfolder such as app or theme.

This issue is to ask whether or not it would possible and desirable to scan the entire module for a package.json file to add to the root project.

What steps will reproduce the problem?

Create a Drupal project and require foxy:

composer create-project drupal/recommended-project test_foxy_collapsiblock
composer require foxy/foxy

Require a module with foxy enabled and package.json below the root (theme folder in this case)

composer require drupal/collapsiblock:4.x-dev#b381e7a

What is the expected result?

Collapsiblock should be built as a dependency in the project's root package.json file:

{
    "license": "GPL-2.0-or-later",
    "dependencies": {
        "@composer-asset/drupal--collapsiblock": "file:./vendor/foxy/composer-asset/drupal/collapsiblock"
    }
}

What do you get instead?

Empty dependencies:

{
    "license": "GPL-2.0-or-later"
}

Additional info

Q A
Version 1.3.0
PHP version 8.1
Operating system Mac OS Sonoma 14.4.1 (23E224)
@darvanen
Copy link
Author

Oops, this is a fork, so sorry.

@darvanen darvanen closed this as not planned Won't fix, can't repro, duplicate, stale May 20, 2024
@terabytesoftw
Copy link
Contributor

Well, i actually maintain this version of the fork, what is the idea, being able to tell foxy which folders to scan, in the cli, or in the configuration?

@terabytesoftw terabytesoftw reopened this May 20, 2024
@terabytesoftw terabytesoftw added the Estado: mejora Nueva función o petición label May 20, 2024
@darvanen
Copy link
Author

darvanen commented May 20, 2024

Yeah I'm coming to the conclusion that this is the one to use, thanks for picking it up.

How about the dependency itself (Collapsiblock in this case) defines that in config.foxy.package-file-path?

@darvanen
Copy link
Author

Aside: what confused me I think is that this documentation still refers to foxy/foxy: https://github.com/php-forge/foxy/blob/main/resources/doc/usage.md

@terabytesoftw
Copy link
Contributor

Aside: what confused me I think is that this documentation still refers to foxy/foxy: https://github.com/php-forge/foxy/blob/main/resources/doc/usage.md

Thks, in a while, resolve the confusion in the document,

@terabytesoftw
Copy link
Contributor

terabytesoftw commented May 20, 2024

Yeah I'm coming to the conclusion that this is the one to use, thanks for picking it up.

How about the dependency itself (Collapsiblock in this case) defines that in config.foxy.package-file-path?

If not specified, it will only go through the root directory, specifying the path, it will go through the root directory, and the paths indicated for that package, i think this would solve the problem.

@darvanen
Copy link
Author

darvanen commented May 20, 2024

Sounds good to me :)

I suppose there might be an edge case where someone has more than one package file 😱 so if it accepts a string or an array of strings I guess that would cover all bases?

Just a string would probably be enough though, not sure I want to encourage that kind of nightmare 🤣

@terabytesoftw
Copy link
Contributor

related fxpio/foxy#5

Maybe this can work.

@darvanen
Copy link
Author

Not really:

For NPM or Yarn, the package.json is must be in the root of your project.

If you want to make this the rule then that's ok, this issue was to find out what the options are

@terabytesoftw
Copy link
Contributor

terabytesoftw commented May 20, 2024

If i understand the problem correctly, we can have in the same package a package.json in root and another in an app directory, for example, or have the package.json in another directory diferent to root, i will review the code.

@darvanen
Copy link
Author

I'm happy for there to be only one location for a package.json file in the library, I think that would actually be best after some thought.

So if a library sets config.foxy.package-file-path then that's the only place where the package.json file should be.

e.g.

{
  "config": {
    "foxy": {
      "package-file-path": "theme"
    }
  }
}

Would mean that the library's package.json file is at ./theme/package.json

@terabytesoftw
Copy link
Contributor

Here we have a problem with download managers, for example in npm if we use --prefix, then it will install within the indicated path, i think a simple and clean solution is for the plugin to copy the package.json to the root folder, and It will install perfectly, and just delete the package.json from the root folder when the package-file-path is set.

@darvanen
Copy link
Author

I thought maybe the configuration could be used in \Foxy\Util\AssetUtil::getPath - line 59?

@terabytesoftw
Copy link
Contributor

I thought maybe the configuration could be used in \Foxy\Util\AssetUtil::getPath - line 59?

Yes it works only for dependent packages, but it should work for both cases when the extension is rootPackage, and when it is a dependent package, in this way the operation will be uniform, i will work on it.

@darvanen
Copy link
Author

darvanen commented May 25, 2024

Thank you!
Any progress you'd like to share? I have some time this weekend I could spend on this if I knew the direction you'd like it to take.

And for the record, my use-case is only for dependent packages, but I can see how it would be useful for the root package too.

@terabytesoftw
Copy link
Contributor

terabytesoftw commented May 27, 2024

Thank you! Any progress you'd like to share? I have some time this weekend I could spend on this if I knew the direction you'd like it to take.

And for the record, my use-case is only for dependent packages, but I can see how it would be useful for the root package too.

You can try the draft PR, it should work.

Example config:

"config": {
    "allow-plugins": {
        "infection/extension-installer": true,
        "composer/installers": true,
        "php-forge/foxy": true
    },
    "foxy": {
        "root-package-dir": "theme"
    },
    "sort-packages": true
},

@darvanen
Copy link
Author

darvanen commented Jun 9, 2024

Champion, thank you! I have high hopes this will enable a big change for the Drupal project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Estado: mejora Nueva función o petición
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants