Update versions in package.json, use @wordpress/scripts for build process #492

kienstra · 2019-12-18T04:06:08Z

There are some packages that are very out of date...I should have been better about updating them.
Updates the packages in package.json to the latest version in https://www.npmjs.com/. For example, https://www.npmjs.com/package/gulp-run.
Also uses the recommended @wordpress/scripts package in the build process. This will make Block Lab more in line with Gutenberg-based plugins.
Adds the package-lock.json file to the repo. This should ensure consistency of versions. For example, in package.json, the gulp version is ^4.0.2. The ^ means that doing npm install can update gulp to 4.1.0 or 4.2.0 if and when those exist. So package-lock.json should ensure that we have the same package tree.

Closes #266

Use the version on npmjs.com as a reference, for example: https://www.npmjs.com/package/npm-run-all

In webpack.config.js, migrate from extract-text-webpack-plugin to mini-css-extract-plugin, as it was deprecated and no longer available.

@see https://make.wordpress.org/core/2019/03/25/building-javascript/ This also required modifying webpack.config.js. It looks like the style.scss or style.css files are not used anywhere, so remove the rules for them.

I'm not sure how I missed these before. But this is simply from running the npm script.

js/blocks/components/advanced-controls.js

webpack.config.js

kienstra · 2019-12-18T04:11:05Z

webpack.config.js

@@ -57,19 +25,42 @@ module.exports = {
 					loader: 'babel-loader',
 				},
 			},
-			{
-				test: /style\.s?css$/,


It looks like this plugin doesn't use style.css or style.scss. I couldn't find a reference to it, though maybe I missed something.

kienstra · 2019-12-18T04:12:12Z

.gitignore

@@ -88,6 +88,7 @@ package-lock.json
 *.swp
 *.tmp
 *.bak
+*.map


The new build process creates .map files, like block-lab/css/blocks.editor.css.map.

kienstra · 2019-12-18T04:18:47Z

To test this, I've been deleting block-lab/css/blocks.editor.css and block-lab/js/editor.blocks.js, and ensuring that npm run dev rebuilds those files as expected.

Also, restore the ^ to some npm packages. This will enable upgrades to minor releases, like from 0.15.0 to 0.16.0.

kienstra · 2019-12-18T04:48:30Z

I still need to do a lot of regression testing here, as it upgrades almost every package in package.json, and refactors the build system.

To be more in line with typical Gutenberg-based plugins.

kienstra · 2019-12-19T00:39:16Z

@todo: set up something like dependabot to automatically open PRs when packages need to be updated.

See #493

I'm not sure if this is needed anymore, as we're mainly using the wordpress/scripts build system.

Thanks to the AMP plugin for this snippet.

js/admin.block-post.js

Something like foo( 'baz', 'bar', ) looks strange. I'm not sure we need to have that. Also, this eslint configuration is now also used for a non-compiled file: admin.block-post.js And it looks like trailing commas in functions are not supported enough in browsers.

kienstra · 2019-12-19T02:10:32Z

.eslintrc

+      "error",
+      {
+        "arrays": "always-multiline",
+        "objects": "always-multiline",


This requires the trailing comma after 'bar' here:

const foo = { one: 'baz', two: 'bar', };

...but doesn't allow a trailing comma after 'bar' here:

const foo = { one: 'baz', two: 'bar' };

…ents Instead of simply allowing this, require it.

kienstra · 2019-12-19T02:15:00Z

.eslintrc

+        "objects": "always-multiline",
+        "imports": "always-multiline",
+        "exports": "always-multiline",
+        "functions": "never"


A trailing comma after the last argument looks strange to me: doThis( 'arg1', 'arg2', )

...and it doesn't have great browser support. This .eslintrc is also used for a non-compiled file: admin.block-post.js.

Ideally, soon that file will be ported to the React logic in the refactoring.

This isn't used anymore, so there's no need to require it.

kienstra · 2019-12-19T02:28:33Z

.eslintrc

+      {
+        "ignoreRestSiblings": true
+      }
+    ]


This file could probably use more rules, but that's not in scope for this PR.

Apply fixes for the errors that this script produces.

Ensures that the dependencies have the same license as this plugin

kienstra · 2019-12-19T04:17:45Z

CSS and JS Compilation

Because this edits webpack.config.js so much, it's important to check that the CSS is compiling correctly to blocks.editor.css.

Here's a diff of the compiled blocks.editor.css file. On the left is the develop branch, and the right is this PR's branch. This is from doing npm install && npm run build on both branches.

Much of the diff is from vendor prefixes:

So far, I didn't see a CSS or JS problem on:

MacOS with Chrome 79
Windows 10 with Edge 18.0 (with BrowserStack)

I tested all fields, including the repeater. It's possible that I missed a styling issue, but I didn't see anything obvious.

kienstra · 2019-12-19T04:33:42Z

Request For Review

Hi @lukecarbis,
Good seeing you earlier. Could you please review this when you have a chance?

It updates the packages in package.json, and uses the Gutenberg-recommended build process: wp scripts start.

Regression testing of all of the fields looks good so far.

Thanks, Luke!

kienstra · 2019-12-19T04:35:49Z

package.json

+    "block-editor",
+    "wordpress",
+    "wordpress-plugin"
+  ],


These edits are from the lint:pkg-json script below.

kienstra · 2019-12-19T04:54:58Z

package.json

@@ -2,50 +2,65 @@
  "name": "block-lab",
  "title": "Block Lab",
  "version": "1.5.2",
+  "description": "WordPress plugin with a simple templating system for building custom blocks.",


Feel free to suggest something different, I added this because there was an error from not having a description.

Thanks to the AMP plugin for the syntax for the plugin. There was an issue reported in the Slack #support channel, where wp.nux.DotTip was moved to wp.components.Guide. By importing WP dependencies as modules, we can control the versions we're using. Also, use a dependency injection plugin. This will keep the dependencies up to date in wp_enqueue_script(). I don't like how it adds a PHP file to js/ But without that, there was a bug, with the console error: Uncaught TypeError: this.activateMode is not a function

There looks to be a second one, though I'm not sure why.

These mentioned that InspectorControls and InspectorAdvancedControls moved to block-editor.

We might consider how to replace this in a separate issue. But for now, I think it's appropriate to remove it.

kienstra · 2020-01-12T17:19:11Z

Just opened #497 to possibly consider an alternative to the DotTip, which is removed in 440f50a

kienstra · 2020-01-12T17:52:51Z

js/blocks/components/fields.js

@@ -1,8 +1,8 @@
 /**
 * WordPress dependencies
 */
-const { applyFilters } = wp.hooks;
-const { select } = wp.data;
+import { applyFilters } from '@wordpress/hooks';


The reason for importing these as modules instead of using something like wp.hooks is so that we can control the module versions.

For example, InspectorControls and InspectorAdvancedControls were moved from wp.editor to wp.block-editor. And wp.nux was deprecated entirely.

If we control the versions of the modules, we should be less dependent on the version of Core and Gutenberg users have.

A user reported an error from wp.nux.DotTips being deprecated in Gutenberg. I probably would have seen that if we imported that deprecated module.

kienstra · 2020-01-12T20:11:40Z

Request For Review

Hi @lukecarbis,
When you have a chance, could you please review this? With the latest commits, it's ready for review.

440f50a in this PR fixes an issue raised in the #support Slack channel, and in https://wordpress.org/support/topic/blocks-not-visible-in-admin-browser-console-error-wp-nux-not-defined/

Regression testing of all fields looks good. Though of course this is a huge diff.

Thanks, Luke!

lukecarbis

TBH I don't understand most of this, but it mostly seems to work. The parts that don't could be my machine. After running npm install, I also tried:

npm run check-licenses ✅
npm run dev ✅
npm run build ✅
npm run lint ❌ (ERROR: Referenced sniff "WordPress-Core" does not exist)
npm run lint:js ✅
npm run lint:pkg-json ✅
npm run lint:php ❌ (ERROR: Referenced sniff "WordPress-Core" does not exist)

lukecarbis · 2020-01-13T04:43:07Z

FWIW, if I run phpcs -i, this is the output:

The installed coding standards are PEAR, Zend, PSR2, MySource, Squiz, PSR1, PSR12, WordPress, WordPress-Extra, WordPress-Docs, WordPress-Core, PHPCompatibility, PHPCompatibilityParagonieRandomCompat, PHPCompatibilityParagonieSodiumCompat and PHPCompatibilityWP

WordPress-Core is obviously installed on my system.

kienstra · 2020-01-13T15:11:52Z

Thanks for bringing that up, it's important that npm run lint works. I'll look at that.

kienstra · 2020-01-13T15:47:22Z

Hi @lukecarbis,
Thanks a lot for reviewing this. Maybe you tried this already, but does that error still exist after running:

rm -rf vendor && composer install && npm run lint:php

There's a package in composer.json that should ensure that WordPress-Core is available.

lukecarbis · 2020-01-13T18:17:51Z

Just tried the following:

$ rm -rf vendor
$ rm -rf composer.lock
$ composer install
$ npm run lint:php

That fixed the problem. Note, I had to also remove the composer.lock file.

Ready to merge!

kienstra · 2020-01-13T19:34:49Z

Ah, thanks. Good idea to remove composer.lock.

kienstra · 2020-01-17T00:57:58Z

Here's a build of the develop branch, probably very close to what the next version will be.

block-lab.zip

kienstra added 4 commits December 17, 2019 16:38

Update npm packages to the latest versions

0c57b81

Use the version on npmjs.com as a reference, for example: https://www.npmjs.com/package/npm-run-all

Update some more npm packages and webpack.config.js

7b643f4

In webpack.config.js, migrate from extract-text-webpack-plugin to mini-css-extract-plugin, as it was deprecated and no longer available.

Use the @wordpress/scripts package to build JS

e6cbebe

@see https://make.wordpress.org/core/2019/03/25/building-javascript/ This also required modifying webpack.config.js. It looks like the style.scss or style.css files are not used anywhere, so remove the rules for them.

Add trailing commas to object values, from doing npm run lint:js:fix

057e2b3

I'm not sure how I missed these before. But this is simply from running the npm script.

kienstra commented Dec 18, 2019

View reviewed changes

js/blocks/components/advanced-controls.js Outdated Show resolved Hide resolved

kienstra commented Dec 18, 2019

View reviewed changes

webpack.config.js Show resolved Hide resolved

kienstra commented Dec 18, 2019

View reviewed changes

Commit package-lock.json, exclude map files from build in gulpfile.js

66e1649

Also, restore the ^ to some npm packages. This will enable upgrades to minor releases, like from 0.15.0 to 0.16.0.

Use wordpress/scripts package for ESLint

4005f48

To be more in line with typical Gutenberg-based plugins.

kienstra added 2 commits December 18, 2019 18:43

Remove UglifyJSPlugin from webpack.config.js

0bb5545

I'm not sure if this is needed anymore, as we're mainly using the wordpress/scripts build system.

Add a rule to .eslintrc of no-unused-vars

88c792d

Thanks to the AMP plugin for this snippet.