Skip to content

(feat): add missing @babel/preset-react dependency and add @Sage alia #44

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.

Sign up for GitHub

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

Merged
YvetteNikolov merged 2 commits into from
Jul 29, 2025
Merged

(feat): add missing @babel/preset-react dependency and add @Sage alia #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d587f1
(feat): add missing @babel/preset-react dependency and add @sage alia…
YvetteNikolov Jul 29, 2025
33b1c26
(feat): remove @wordpress/scripts
YvetteNikolov Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30,351 changes: 11,231 additions & 19,120 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

53 changes: 28 additions & 25 deletions packages/eslint-config/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,30 @@
{
"name": "@yardinternet/eslint-config",
"version": "1.0.1",
"description": "Eslint configuration",
"main": "src/index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"publishConfig": {
"registry": "https://npm.pkg.github.com/"
},
"repository": {
"type": "git",
"url": "git+https://github.com/yardinternet/toolkit.git",
"directory": "packages/eslint-config"
},
"type": "commonjs",
"author": "",
"license": "ISC",
"dependencies": {
"@eslint/eslintrc": "^3.2.0",
"@eslint/js": "^9.17.0",
"@wordpress/eslint-plugin": "^22.1.1",
"eslint-config-prettier": "^9.1.0",
"globals": "^15.14.0"
}
"name": "@yardinternet/eslint-config",
"version": "1.0.1",
"description": "Eslint configuration",
"main": "src/index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"publishConfig": {
"registry": "https://npm.pkg.github.com/"
},
"repository": {
"type": "git",
"url": "git+https://github.com/yardinternet/toolkit.git",
"directory": "packages/eslint-config"
},
"type": "commonjs",
"author": "",
"license": "ISC",
"dependencies": {
"@babel/preset-react": "^7.27.1",
"@eslint/eslintrc": "^3.2.0",
"@eslint/js": "^9.17.0",
"@wordpress/eslint-plugin": "^22.1.1",
"eslint-config-prettier": "^9.1.0",
"eslint-import-resolver-alias": "^1.1.2",
"eslint-plugin-import": "^2.32.0",
"globals": "^15.14.0"
}
}
11 changes: 11 additions & 0 deletions packages/eslint-config/src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,16 @@ module.exports = [
},
],
},

settings: {
'import/resolver': {
alias: [
Copy link
Copy Markdown
Contributor

@FreakyWizard FreakyWizard Jul 29, 2025

Choose a reason for hiding this comment

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

Dit is neem ik aan zodat hij Vite begrijpt? Ik neem aan dat het geen probleem is als deze config in een project met webpack wordt gebruikt, tock?

Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025

Choose a reason for hiding this comment

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

Ook in oude projecten gaat dit fout en gaat de linter kapot

Dit is bijv. hier https://github.com/yardinternet/brave-scaffold/blob/main/stubs/sage-child/resources/scripts/frontend/frontend.js

[
'@sage/scripts',
'./web/app/themes/sage-child/resources/scripts',
],
],
},
},
},
];
66 changes: 36 additions & 30 deletions packages/toolkit/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
{
"name": "@yardinternet/toolkit",
"version": "1.0.3",
"description": "Yard toolkit CLI scripts",
"bin": {
"yard-toolkit": "./src/index.js"
},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"publishConfig": {
"registry": "https://npm.pkg.github.com/"
},
"repository": {
"type": "git",
"url": "git+https://github.com/yardinternet/toolkit.git",
"directory": "packages/toolkit"
},
"type": "module",
"author": "",
"license": "ISC",
"dependencies": {
"@wordpress/scripts": "^30.9.0",
"chalk": "^5.4.1",
"meow": "^13.2.0"
},
"optionalDependencies": {
"@yardinternet/eslint-config": "^1.0.1",
"@yardinternet/prettier-config": "^1.0.2",
"@yardinternet/stylelint-config": "^1.0.1"
}
"name": "@yardinternet/toolkit",
"version": "1.0.3",
"description": "Yard toolkit CLI scripts",
"bin": {
"yard-toolkit": "./src/index.js"
},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"publishConfig": {
"registry": "https://npm.pkg.github.com/"
},
"repository": {
"type": "git",
"url": "git+https://github.com/yardinternet/toolkit.git",
"directory": "packages/toolkit"
},
"type": "module",
"author": "",
"license": "ISC",
"dependencies": {
"@babel/preset-react": "^7.27.1",
"chalk": "^5.4.1",
"eslint": "^9.32.0",
"meow": "^13.2.0",
"prettier": "npm:wp-prettier@^3.0.3",
Copy link
Copy Markdown
Contributor

@FreakyWizard FreakyWizard Jul 29, 2025

Choose a reason for hiding this comment

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

Was dit dus toch nodig voor de override? Of is dit voor de verbositeit?

Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025

Choose a reason for hiding this comment

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

Nu nodig omdat we prettier direct gebruiken in het format.js bestand

Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025

Choose a reason for hiding this comment

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

Nu je het zegt, kunnen we de overrides weghalen. Ga ik doen.

"stylelint": "^16.23.0"
},
"optionalDependencies": {
"@yardinternet/eslint-config": "*",
Copy link
Copy Markdown
Contributor

@FreakyWizard FreakyWizard Jul 29, 2025

Choose a reason for hiding this comment

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

Is any version wel handig? Zo meteen ontstaan er in de toekomst compatibiliteit problemen in oude projecten (een oude versie an de Toolkit CLI maar een nieuwe config). Anderzijds is het wel handig dat het dan automatisch 'geüpdatet' zonder dat je een nieuwe versie moet uitbrengen.

Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025

Choose a reason for hiding this comment

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

Ja dat laatste. Als de toolkit geupdatet wordt in een project, doen we dat vaak via zo'n command die alle @yardinternet/ dependencies updatet.

Het is dan apart dat hier bij optionalDependencies een mogelijk oude versie staat.

"@yardinternet/prettier-config": "*",
"@yardinternet/stylelint-config": "*"
},
"overrides": {
"prettier": "npm:wp-prettier@^3.0.3"
}
}
24 changes: 21 additions & 3 deletions packages/toolkit/src/actions/format.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,38 @@
import { spawn } from 'child_process';

import {
filetypeFromString,
getGlobByFormatModeAndFiletype,
modesFromString,
run,
error,
} from '../utils/helpers.js';

export const format = ( options, filetype, userPath ) => {
const formatFiletype = filetypeFromString( filetype, true );
const formatMode = modesFromString( options.mode, true );

const command = 'wp-scripts format';
const command = 'prettier';

const glob = getGlobByFormatModeAndFiletype(
formatMode,
formatFiletype.name
);

run( `${ command } ${ glob?.path ?? '' } ${ userPath ?? '' }`, 'format' );
const args = [
...( glob?.path ? [ glob.path ] : [] ),
...( userPath ? [ userPath ] : [] ),
'--check',
'--write',
];
Comment on lines +14 to +26
Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Dit was trouwens het probleem waar we eerder tegenaan liepen. Met run( ${ command } ${ glob?.path ?? '' } ${ userPath ?? '' }, 'format' ); werden de aanhalingstekens niet meegenomen:

prettier ./web/app/themes/**/resources/styles/{*,**/*}.css --write

Terwijl we die wel nodig hadden voor het command

prettier './web/app/themes/**/resources/styles/{*,**/*}.css' --write

Met spawn() en het gebruik van een args array worden de argumenten nu wel goed doorgegeven.

Copy link
Copy Markdown
Contributor

@FreakyWizard FreakyWizard Jul 29, 2025

Choose a reason for hiding this comment

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

Interessant, glob?.path is toch gewoon een string. Dus '${ glob?.path ?? '' }' had denk ik ook kunnen werken.
Overigens deze manier (met een args array) lijkt mij veel netter!

Wellicht kan deze logica nog weg geabstraheerd worden naar een helper of wellicht in run (of niet :) )

Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025

Choose a reason for hiding this comment

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

Nope, '${ glob?.path ?? '' }' met aanhalingstekens werkt ook niet - de aanhalingstekens werden gestript


const child = spawn( command, args, {
Copy link
Copy Markdown
Contributor

@FreakyWizard FreakyWizard Jul 29, 2025

Choose a reason for hiding this comment

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

Is het niet beter om de exec in de run helper gewoon te vervangen voor de deze spawn functie? De functionaliteit is toch het zelfde? Alleen de logs worden wel getoond

Copy link
Copy Markdown
Contributor

@FreakyWizard FreakyWizard Jul 29, 2025

Choose a reason for hiding this comment

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

Laat maar ik zie nu je opmerking hierover: https://github.com/yardinternet/toolkit/pull/44/files#r2239860906

stdio: 'inherit',
shell: true, // required for glob support like {*,**/*}.css
} );

child.on( 'exit', ( code ) => {
if ( code !== 0 ) {
error( `Prettier exited with code ${ code }` );
}
} );
Comment on lines +21 to +37
Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025

Choose a reason for hiding this comment

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

Ik ga dit nog herschrijven, want dubbele code met lint.js, komt in de andere PR nadat deze gemerged is

};
27 changes: 19 additions & 8 deletions packages/toolkit/src/actions/lint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { spawn } from 'child_process';

import {
error,
filetypeFromString,
getGlobByFormatModeAndFiletype,
modesFromString,
run,
} from '../utils/helpers.js';
import { filetypes } from '../config/filetypes.js';
import { options as configOptions } from '../config/options.js';
Expand All @@ -21,7 +22,7 @@ export const lint = ( options, filetype, userPath ) => {
break;
case filetypes.scss.name:
case filetypes.css.name:
command = 'wp-scripts lint-style';
command = 'stylelint';
break;
default:
error(
Expand All @@ -34,10 +35,20 @@ export const lint = ( options, filetype, userPath ) => {
formatFiletype.name
);

run(
`${ command } ${ glob?.path ?? '' } ${ userPath ?? '' } ${
isFix ? '--fix' : ''
}`,
'lint'
);
const args = [
...( glob?.path ? [ glob.path ] : [] ),
...( userPath ? [ userPath ] : [] ),
...( isFix ? [ '--fix' ] : [] ),
];

const child = spawn( command, args, {
stdio: 'inherit',
shell: true, // Required for glob support
Copy link
Copy Markdown
Contributor

@FreakyWizard FreakyWizard Jul 29, 2025

Choose a reason for hiding this comment

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

interessant dat je daar een optie voor moet mee geven. Ik zal denken dat dat gewoon binnen de 'stylelint' code wordt afgehandeld

Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

https://nodejs.org/api/child_process.html#child_processspawncommand-args-options

spawn() gebruikt standaard geen shell en in de shell worden de glob patterns geïnterpreteerd. Als ik dit niet zou doorgeven, zal de glob letterlijk doorgegeven worden als string.

} );

child.on( 'exit', ( code ) => {
if ( code !== 0 ) {
error( `${ command } exited with code ${ code }` );
}
} );
};
2 changes: 1 addition & 1 deletion packages/toolkit/src/config/modes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const modes = {
},
{
filetype: filetypes.css.name,
path: './web/app/themes/**/resources/styles/**/*.css',
path: './web/app/themes/**/resources/styles/{*,**/*}.css',
Copy link
Copy Markdown
Contributor Author

@YvetteNikolov YvetteNikolov Jul 29, 2025

Choose a reason for hiding this comment

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

Met path: './web/app/themes/**/resources/styles/{*,**/*}.css', pakte hij ./web/app/themes/**/resources/styles/frontend.css niet. Met deze glob nu wel.

},
],
},
Expand Down