DEV-37: Use eslint to lint custom javascript #146

jesconstantine · 2023-06-07T19:14:30Z

User story: DEV-37: Use Drupal core JS and CSS linting as default

Description

This PR adds the necessary configuration and scripting to use eslint on our custom js in projects.

Testing instructions

Run composer create-project palantirnet/drupal-skeleton test dev-dev-37-eslint
Follow the updated readme "Quick Start" steps #2-3 to get Drupal installed in your test project: https://github.com/palantirnet/drupal-skeleton/pull/146/files?short_path=b335630#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5
Exit your ddev ssh session
Run ddev phing code-review from your host machine
- Observe that we check for npm dependencies being installed at 2 levels: project and in core.
- Observe that we install the dependencies.
- Observe that the code-review passes

Add some bad js to ${drupal.root}/modules/custom/bad-js/bad.es6.js:

	 const squared = x * x;
	 const cubed = x * x * x;
	 console.log('squared =', squared);
	 console.log('cubed =', cubed);

	 const squared2 = Math.pow(x, 2);
	 const cubed2 = Math.pow(x, 3);
	 console.log('squared2 =', squared2);
	 console.log('cubed2 =', cubed2);

	 const complex = Math.pow(x * 2, 3 + 4);
	 console.log('complex =', complex);

Run ddev phing code-review from your host machine
- Observe that we check for npm dependencies being installed at 2 levels: project and in core.
- Observe that we no longer installed the dependencies because we found them already.
- Observe that the PHP linters still pass
- Observe that eslint finds errors
- Observe the step fails as it should.

Notes

This PR relies on palantirnet/the-build#223

README.md

jesconstantine · 2023-06-07T20:56:12Z

.nvmrc

-12
+18


From #144 - thanks Byron!

jesconstantine · 2023-06-07T20:56:25Z

.circleci/config.yml

-      - NODE_VERSION: 8
+      - NODE_VERSION: 18


From #144 - thanks Byron!

jesconstantine · 2023-06-07T21:01:50Z

README.md

-  ```
-  composer create-project palantirnet/drupal-skeleton example dev-develop --no-interaction
-  ```
-
-  This skeleton is based on Drupal 10. If you would like to install and use Drupal 9 instead, run:
-  ```
-  composer create-project palantirnet/drupal-skeleton example dev-drupal9 --no-interaction
-  ```
-
-2. Go into your new project directory and update the ddev configuration in `.ddev/config.yml`:
+    ```
+    composer create-project palantirnet/drupal-skeleton example dev-develop --no-interaction
+    ```
+
+    This skeleton is based on Drupal 10. If you would like to install and use Drupal 9 instead, run:


These changes are all just formatting to allow us to just use 1.

jesconstantine · 2023-06-07T21:02:16Z

README.md

+Update the `package.json`:
+
+* Change the `name` from `palantirnet--drupal-skeleton` to `palantirnet--PROJECTNAME`
+* Update the `description` with a brief description of your project.
+


This is a new addition.

jesconstantine · 2023-06-07T21:02:48Z

composer.json

+        },
+        {
+            "type": "git",
+            "url": "https://github.com/palantirnet/the-build"


This is temporary. After this PR is approved we should be able to remove.

jesconstantine · 2023-06-07T21:03:07Z

composer.json

-        "palantirnet/the-build": "^4@beta"
+        "palantirnet/the-build": "dev-dev-37-eslint"


This is temporary. After this PR is approved we should be able to revert or update.

.ddev/commands/web/npx

jesconstantine · 2023-06-07T21:04:19Z

.ddev/commands/web/phing

+#!/bin/bash
+
+## Description: Run phing inside the shell with `ddev phing`
+
+vendor/bin/phing "$@"


Allows us to run ddev phing ...

jesconstantine · 2023-06-08T17:20:30Z

.eslintrc.json

@@ -0,0 +1,5 @@
+{
+  "extends": [
+    "drupal"


We just use Drupal Core's rules and config for eslint.

This reverts commit 33f184e.

We were able to remove the node dependency in favor of just using yarn ++

patrickfweston

Everything here works great for me! I think this will help us out a lot on DHS, so I'd be in favor of merging this in. I'm going to hold off until others have a chance to look though.

patrickfweston · 2023-08-07T19:45:54Z

I do see there is a test failure, but I'm wondering if we can test that in a different way. I'm not sure how relevant the existing test is.

This reverts commit 24d75fd.

jesconstantine added 3 commits June 7, 2023 12:20

Add scaffolding for eslint to the skeleton

cb63310

Use node v18 in nvmrc

871ac81

Update docs with node version, npm context

b22f08b

jesconstantine changed the title ~~Use eslint to lint custom javascript~~ DEV-37: Use eslint to lint custom javascript Jun 7, 2023

jesconstantine added 9 commits June 7, 2023 13:30

Update circle to use node v 18

da5009c

TEMPORARY: pin the-build to our dev branch version for eslint

ee72af2

Remove illegal character from package.json name

fa889dd

Add node package to check dependencies, used in phing target

a4f1310

Update readme

4bb74e7

Add npx and phing commands to ddev

797638f

Fix readme formatting for easier "quick setup" ordered list coding

7fa63f0

Remove unneeded new setup step

ced6bd7

Format alltheorderedlists with 1.

8914717

jesconstantine commented Jun 7, 2023

View reviewed changes

.ddev/commands/web/npx Outdated Show resolved Hide resolved

jesconstantine commented Jun 7, 2023

View reviewed changes

jesconstantine marked this pull request as ready for review June 7, 2023 21:04

jesconstantine mentioned this pull request Jun 7, 2023

DEV-37: Lint custom javascript with eslint palantirnet/the-build#223

Merged

jesconstantine commented Jun 8, 2023

View reviewed changes

.eslintrc.json

@@ -0,0 +1,5 @@

{

"extends": [

"drupal"

Copy link

Contributor Author

jesconstantine Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just use Drupal Core's rules and config for eslint.

jesconstantine added 9 commits June 26, 2023 09:58

Remove project-level dependencies

33f184e

Revert "Remove project-level dependencies"

98f18f0

This reverts commit 33f184e.

Remove unneeded package dependencies

8be8573

Remove legacy ddev web container command for npx

ffc899f

We were able to remove the node dependency in favor of just using yarn ++

Remove reference to npm as a dependency since it's in ddev already

b14ca24

Add link to package.json name convention requirements

e7a2d23

Try removing eslint yaml plugin from project root package.json

2cb502b

Reference eslint configuration in the-build

ca06fcb

Remove node as a project dependency - it ships on ddev.

893b054

patrickfweston self-requested a review August 7, 2023 19:14

patrickfweston approved these changes Aug 7, 2023

View reviewed changes

agentrickard and others added 3 commits August 16, 2023 16:18

Use php 8.1.18

24d75fd

Merge remote-tracking branch 'origin/develop' into dev-37-eslint

74b3e6e

Revert "Use php 8.1.18"

19885d8

This reverts commit 24d75fd.

byrond merged commit d525245 into develop Aug 16, 2023

byrond deleted the dev-37-eslint branch August 16, 2023 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEV-37: Use eslint to lint custom javascript #146

DEV-37: Use eslint to lint custom javascript #146

jesconstantine commented Jun 7, 2023 •

edited by patrickfweston

Loading

jesconstantine Jun 7, 2023

jesconstantine Jun 7, 2023

jesconstantine Jun 7, 2023

jesconstantine Jun 7, 2023

jesconstantine Jun 7, 2023

jesconstantine Jun 7, 2023

jesconstantine Jun 7, 2023

jesconstantine Jun 8, 2023

patrickfweston left a comment

patrickfweston commented Aug 7, 2023

		"palantirnet/the-build": "^4@beta"
		"palantirnet/the-build": "dev-dev-37-eslint"

DEV-37: Use eslint to lint custom javascript #146

DEV-37: Use eslint to lint custom javascript #146

Conversation

jesconstantine commented Jun 7, 2023 • edited by patrickfweston Loading

Description

Testing instructions

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickfweston left a comment

Choose a reason for hiding this comment

patrickfweston commented Aug 7, 2023

jesconstantine commented Jun 7, 2023 •

edited by patrickfweston

Loading