Improved composer.json file by JulienMelissas · Pull Request #1229 · WordPress/gutenberg

JulienMelissas · 2017-06-17T15:30:12Z

Added more information, the "wordpress-plugin" package type, and other required things to make installing the plugin via composer easier.

Of course the plugin will need to get registered on packagist but this gets us started.

I also would like to double-check the license is GPL2 vs 3 (see here) and then I will add a PR for a license.md file.

Cheers 🍻

Added more information, the "wordpress-plugin" package type, and other required things to make installing the plugin via composer easier.

youknowriad · 2017-06-17T15:39:56Z

Thanks for the PR.

I don't think it was a goal to make the plugin installable via composer. The composer.json is here only for dev purpose as a handy way to get phpcs maybe, I guess. Worth noting the plugin is not ready to use if you just grab the composer package, you'll need to build it first.

Is there a way to define a prepublish build step for composer? Do we really expect this plugin to be installed using composer?

westonruter · 2017-06-17T15:42:42Z

@youknowriad

Is there a way to define a prepublish build step for composer? Do we really expect this plugin to be installed using composer?

Yes. Actually, there is one already. There is a post-install-cmd which installs WordPress-Coding-Standards. So there could be another one which does npm install, potentially.

JulienMelissas · 2017-06-18T11:09:25Z

@youknowriad - I sorry, I didn't realize you don't include the build files in the base repo. I looked for a few minutes for that explanation somewhere in the docs, how to get started or build the files, but I I don't see that. Would you like me to add some documentation for that as well? Anyways, I'll need to update this PR in order for this to work regardless.

Like @westonruter said we could use a post-install-cmd to run npm install or yarn install and whatever build processes needed. In my opinion someday it would be great to see most WordPress plugins available via composer - it gives you a bit more support on versions/releases as well as more reliability over wppackagist. Basically I'm happy to put in the work to get better composer support but if you don't think it's needed feel free to close this out. Thanks!

aduth · 2017-06-20T18:00:58Z

CONTRIBUTING.md includes some information about building the plugin from source files. We could perhaps do a better job of highlighting the fact that the source files are not usable out-of-the-box.

JulienMelissas · 2017-06-21T17:16:08Z

I feel like it's totally possible to do a post-install, but if you all think this might be too much to expect out of this repo (for now), I'm okay with a close and I'll just install from wpackagist.

GaryJones · 2017-06-22T07:31:56Z

I'd say the License should be GPL-2.0+.

Here's a potential improvement to the scripts:

"scripts": {
    "build": "npm install",
    "install-standards": "\"vendor/bin/phpcs\" --config-set installed_paths ../../wp-coding-standards/wpcs",
    "post-install-cmd": [
      "@install-standards",
      "@build"
    ]
  }

Local path given for phpcs, since a project should not be reliant on something being installed globally.
Commands decoupled from triggers, so they can be run individually i.e. composer run install-standards instead of only being able to do that as part of a composer install. Also makes them more self-documenting, and keeps them DRY if a post-update-cmd trigger was later added with them too.

JulienMelissas · 2017-06-23T01:05:26Z

@GaryJones great point on the composer commands - wouldn't we want "build" to run npm install && npm run build?

GaryJones · 2017-06-23T10:06:21Z

wouldn't we want "build" to run npm install && npm run build

Er...yup :-)

You could probably have the install (or update) and build on post-update-cmd as well.

gziolo · 2017-09-22T12:56:50Z

Composer could be better explained in our docs. In particular for those who come from JavaScript world. We might also consider providing a two way integration between JavaScript and PHP. Similar to what is proposed above for Composer, we could add an alternative command which would bootstrap composer :) It might be too confusing though, but we can always discuss pros and cons.

GaryJones · 2017-11-04T10:38:45Z

+    "dealerdirect/phpcodesniffer-composer-installer": "^0.4",
+    "squizlabs/php_codesniffer": "2.9.x",
+    "wimg/php-compatibility": "dev-master",
+    "wp-coding-standards/wpcs": "^0.11.0"


The latest released version of WPCS is 0.14.0, so ideally this would be the minimum version here.

Fixed by 4e4055f

GaryJones · 2017-11-04T10:39:35Z

+  "scripts": {
+    "lint": "phpcs",
+    "post-install-cmd": [
+      "phpcs --config-set installed_paths ../../wp-coding-standards/wpcs/"


With the DealerDirect plugin, this script is no longer needed (and it doesn't register PHPCompatibility standard anyway).

Removed in 7247134

atimmer · 2017-12-03T21:17:18Z

You cannot assume that npm is available in the environment where composer install is run. If that is added I think it should ignore the npm install if npm is not available.

codecov · 2017-12-03T21:21:43Z

Codecov Report

Merging #1229 into master will decrease coverage by 6.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1229      +/-   ##
==========================================
- Coverage    37.7%   31.51%   -6.19%     
==========================================
  Files         279      238      -41     
  Lines        6737     6686      -51     
  Branches     1226     1199      -27     
==========================================
- Hits         2540     2107     -433     
- Misses       3536     3849     +313     
- Partials      661      730      +69

Impacted Files	Coverage Δ
blocks/url-input/button.js	`0% <0%> (-100%)`	⬇️
components/panel/row.js	`0% <0%> (-100%)`	⬇️
components/notice/index.js	`0% <0%> (-100%)`	⬇️
components/slot-fill/provider.js	`5% <0%> (-85.33%)`	⬇️
components/slot-fill/fill.js	`7.14% <0%> (-80.86%)`	⬇️
components/slot-fill/slot.js	`7.14% <0%> (-72.03%)`	⬇️
blocks/autocompleters/index.js	`0% <0%> (-42.11%)`	⬇️
blocks/url-input/index.js	`1.63% <0%> (-19%)`	⬇️
...ponents/higher-order/with-spoken-messages/index.js	`66.66% <0%> (-16.67%)`	⬇️
editor/actions.js	`39.13% <0%> (-8.04%)`	⬇️
... and 287 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88af5a9...7d0fd16. Read the comment docs.

JulienMelissas · 2017-12-03T21:34:12Z

@atimmer - makes sense. I'll write a check to see if npm is installed and if not will return a message on post install. Sorry about the failing Travis build as well, that will be fixed in my next commits.

See WordPress#1229 (comment)

JulienMelissas · 2017-12-03T22:19:52Z

Okay. I'm still working on the script to check for npm before running an npm install.

Got another question for you though. Should we run npm install && npm run-script build on post update as well (post-update-cmd)? That would ensure every time someone updates their dependencies (gutenberg being one of them), it would have the latest version of the build files. I think so, but would like a second opinion on edge cases. Thanks.

GaryJones · 2017-12-03T23:12:11Z

+    "issues": "https://github.com/WordPress/gutenberg/issues"
+  },
+  "require-dev": {
+    "dealerdirect/phpcodesniffer-composer-installer": "^0.4",


The early 0.4.* releases had a couple of bugs. The constraint should allow the most recent anyway, but worth specifying ^0.4.3 just to be sure?

GaryJones · 2017-12-03T23:14:23Z

+  },
+  "require-dev": {
+    "dealerdirect/phpcodesniffer-composer-installer": "^0.4",
+    "squizlabs/php_codesniffer": "2.9.x",


What was the reason for staying on the 2.9.x branch?

FWIW, the recent patch that fixed a load of code standards in WP core relied on 3.2.0-dev (master) for PHPCS.

WPCS, PHPCompatibility and the DealerDirect plugin work fine with the PHPCS 3.1.1 (stable release).

A constraint of ^3.1 would be fine.

GaryJones · 2017-12-03T23:16:54Z

+  "require-dev": {
+    "dealerdirect/phpcodesniffer-composer-installer": "^0.4",
+    "squizlabs/php_codesniffer": "2.9.x",
+    "wimg/php-compatibility": "dev-master",


The current release is 8.0.1, and it now follows semantic versioning, so maybe go with ^8 as the version constraint here?

GaryJones · 2017-12-03T23:18:51Z

+    "composer/installers": "~1.0"
+  },
+  "scripts": {
+    "lint": "phpcs"


PHPCS does more than lint, and adding a script that then expects composer lint, instead of just the single command phpcs seems a little redundant.

Totally agreed. I'll remove it.

JulienMelissas · 2017-12-04T06:01:57Z

Thanks to @GaryJones for the recent updates and code checks. Those shouldn't be too much effort at all to update, many of the specific versions were just left over from my initial PR, which was during WordCamp Europe, so totally outdated.

That said, I've thought a bit more about this PR on the drive home today. A script to check for npm install would be needed for sure, so that the build can happen after an install or update, but to ensure a cross-platform solution (macOS, linux, windows) might be difficult. I'm still doing research on how to pull it off. While it might be beneficial to pull Gutenberg via composer, it is possible via wp-packagist where it's already built. It's honestly not super common in my experience using composer for it to even run a npm install.

I suppose what I'm asking is: is the npm building/composer require-ability of this PR even worth it, or is there a better way to solve this problem?

Thanks!

GaryJones · 2017-12-04T07:31:43Z

I suppose what I'm asking is: is the npm building/composer require-ability of this PR even worth it, or is there a better way to solve this problem?

I think there is enough value in the current PR to consider it done (with amendments), and leave the NPM stuff to another PR.

gziolo · 2017-12-04T07:41:21Z

Got another question for you though. Should we run npm install && npm run-script build on post update as well (post-update-cmd)? That would ensure every time someone updates their dependencies (gutenberg being one of them), it would have the latest version of the build files. I think so, but would like a second opinion on edge cases. Thanks.

There are pre and post hooks for npm run-script commands. So you can achieve exectly the same goal by adding "postinstall": "npm run build" or "prebuild": "npm install" to package.json file. Given that you can use npm run build or npm run dev to generate build, so I guess pre commands fit better here.

gziolo · 2017-12-04T07:43:01Z

This leads to another question, should we integrate composer install and build as a npm postinstall hook? :)

JulienMelissas · 2017-12-04T19:54:31Z

@GaryJones looks like that last build didn't work because:

ERROR: Referenced sniff "PHPCompatibility" does not exist

But I ran everything locally and follow these instructions just to double-check. Seems to work great.

Any thoughts? Maybe Travis has some type of cache? Or did I actually miss a step?

Thanks.

JulienMelissas · 2017-12-04T20:01:20Z

As for the other stuff...

I think there is enough value in the current PR to consider it done (with amendments), and leave the NPM stuff to another PR.

I've made those amendments. I totally agree with you. However I think we should have some kind of note that the build files are not build on composer install so that we don't get a bunch of issues like "doesn't work on composer install" - as it won't work ootb.

Good points above @gziolo. I think running from package.json would be fine. There are many ways to skin this cat. But I still think we need to think hard about how to make that work for everyone. Once this PR is merged in with the basic changes I'm happy to open a new ticket to brainstorm how we might accomplish this.

This leads to another question, should we integrate composer install and build as a npm postinstall hook? :)

Good question! Maybe a topic for another issue?

GaryJones · 2017-12-05T13:02:39Z

Maybe Travis has some type of cache?

It seems it does.

The DealerDirect plugin should re-register the standards on composer install. I'm unsure why it didn't here (if not related to the cache). cc @Frenk, @Potherca.

One workaround (well, documented feature that says it's for CIs) would be to add a manual script to composer.json for registering the sniffs:

{
    "scripts": {
        "install-codestandards": [
            "Dealerdirect\\Composer\\Plugin\\Installers\\PHPCodeSniffer\\Plugin::run"
        ]
    }
}

Then, in .travis.yml, after the composer install but before the ./vendor/bin/phpcs here, do a composer install-codestandards.

ntwb · 2017-12-09T04:27:54Z

As all efforts to restart the failing Travis job have failed I added a commit to change the license to GPL-2.0+ per @GaryJones comment above.

Following #3837 Travis CI no longer caches Composer's /vendor folder

Edit: All Travis CI jobs are now passing 📗

GaryJones · 2017-12-09T12:48:41Z

Who needs eyes on this to merge it?

(And why does the master branch not need a review to be signed off before merge?)

ntwb · 2017-12-09T13:26:46Z

(And why does the master branch not need a review to be signed off before merge?)

As the project nears merging with WP Core I'm sure this policy will change...

JulienMelissas · 2017-12-12T07:58:56Z

Thanks for everyone's work on this! I will keep thinking about how to run an automatic npm install and build on post-install/post-update. For now this is at least a good place to start.

Should we mention something in the README about needing to build npm stuff if you use composer to install this as a plugin? (like in Bedrock or something like that?)

Improved composer.json file

d590c6b

Added more information, the "wordpress-plugin" package type, and other required things to make installing the plugin via composer easier.

Merge branch 'master' into better-composer-support

44b30ad

GaryJones reviewed Nov 4, 2017

View reviewed changes

GaryJones suggested changes Nov 4, 2017

View reviewed changes

Merge branch 'master' into better-composer-support

7d0fd16

JulienMelissas added 2 commits December 3, 2017 16:00

Removing the PHPCS config set Post Install Command

7247134

See WordPress#1229 (comment)

Updating WPCS dependency

4e4055f

See WordPress#1229 (comment)

GaryJones reviewed Dec 3, 2017

View reviewed changes

Updated dev dependencies, removed redundant "lint" script.

0beea63

Update license to GPL-2.0+

563aa69

ntwb approved these changes Dec 9, 2017

View reviewed changes

ntwb merged commit 1501844 into WordPress:master Dec 9, 2017

snyk-bot mentioned this pull request Nov 18, 2020

[Snyk] Fix for 1 vulnerabilities wpscholar/gutenberg#12

Open

snyk-bot mentioned this pull request Sep 15, 2021

[Snyk] Fix for 1 vulnerabilities Verdinjoshua26/gutenberg#20

Open

Verdinjoshua26 mentioned this pull request Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities Verdinjoshua26/gutenberg#57

Open

Verdinjoshua26 mentioned this pull request Dec 25, 2022

[Snyk] Fix for 1 vulnerabilities Verdinjoshua26/gutenberg#59

Open

Verdinjoshua26 mentioned this pull request Jun 20, 2023

[Snyk] Fix for 1 vulnerabilities Verdinjoshua26/gutenberg#71

Open

Verdinjoshua26 mentioned this pull request Nov 27, 2023

[Snyk] Fix for 5 vulnerabilities Verdinjoshua26/gutenberg#93

Open

Verdinjoshua26 mentioned this pull request May 14, 2024

[Snyk] Fix for 2 vulnerabilities Verdinjoshua26/gutenberg#121

Open

tyxla mentioned this pull request Jun 10, 2026

List View block support: Hide list tab when allowedBlocks is empty, with no children #78932

Merged

Uh oh!

Conversation

JulienMelissas commented Jun 17, 2017

Uh oh!

youknowriad commented Jun 17, 2017

Uh oh!

westonruter commented Jun 17, 2017

Uh oh!

JulienMelissas commented Jun 18, 2017

Uh oh!

aduth commented Jun 20, 2017

Uh oh!

JulienMelissas commented Jun 21, 2017

Uh oh!

GaryJones commented Jun 22, 2017

Uh oh!

JulienMelissas commented Jun 23, 2017

Uh oh!

GaryJones commented Jun 23, 2017

Uh oh!

gziolo commented Sep 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atimmer commented Dec 3, 2017

Uh oh!

codecov Bot commented Dec 3, 2017

Codecov Report

Uh oh!

JulienMelissas commented Dec 3, 2017

Uh oh!

JulienMelissas commented Dec 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaryJones Dec 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JulienMelissas commented Dec 4, 2017

Uh oh!

GaryJones commented Dec 4, 2017

Uh oh!

gziolo commented Dec 4, 2017

Uh oh!

gziolo commented Dec 4, 2017

Uh oh!

JulienMelissas commented Dec 4, 2017

Uh oh!

JulienMelissas commented Dec 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GaryJones commented Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntwb commented Dec 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GaryJones commented Dec 9, 2017

Uh oh!

ntwb commented Dec 9, 2017

Uh oh!

JulienMelissas commented Dec 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

GaryJones Dec 3, 2017 •

edited

Loading

JulienMelissas commented Dec 4, 2017 •

edited

Loading

GaryJones commented Dec 5, 2017 •

edited

Loading

ntwb commented Dec 9, 2017 •

edited

Loading