Skip to content

Add ESLint configuration and refactor grefplus to use ES modules #169

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add ESLint configuration and refactor grefplus to use ES modules #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
751ab5c
Refactor grefplus to use ES modules and update alias paths
begin-again Oct 15, 2024
628cd6c
Update ESLint configuration for ES module support and bump version to…
begin-again Oct 15, 2024
26ad0bc
Refactor date validation logic in cmdline.mjs to use a reusable function
begin-again Oct 15, 2024
97a6905
Update GitHub Actions workflows to use 'yarn install --immutable' for…
begin-again Oct 17, 2024
ed325f1
Update GitHub Actions workflow to enable corepack and use 'yarn insta…
begin-again Oct 17, 2024
08d52cf
Refactor code structure for improved readability and maintainability
begin-again Oct 17, 2024
73d53f1
Enable corepack in GitHub Actions workflow and update yarn install co…
begin-again Oct 17, 2024
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
8 changes: 8 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ module.exports = {
, mocha: true
, es2020: true
}
, overrides: [
{
files: [ '*.mjs' ]
, parserOptions: {
sourceType: 'module'
}
}
]
, rules: {
'array-bracket-spacing': [ 'error', 'always' ]
, 'arrow-spacing': [ 'error', { 'before': true, 'after': true } ]
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/node.nix.js.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,16 @@ jobs:

steps:
- uses: actions/checkout@v4

- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
- run: yarn install --frozen-lockfile

# Enable corepack
- name: Enable corepack
run: |
corepack enable

- run: yarn install --immutable

- run: yarn test
20 changes: 11 additions & 9 deletions .github/workflows/node.win.js.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# This workflow will do a clean installation of node dependencies, cache/restore them, build the source code and run tests across different versions of node
# For more information see: https://help.github.com/actions/language-and-framework-guides/using-nodejs-with-github-actions

name: Node.win.js CI

on:
Expand All @@ -12,20 +9,25 @@ on:
jobs:
build:
name: Build and Test Windows
runs-on: windows-2019
runs-on: windows-2019

strategy:
matrix:
node-version: [20.x]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/

env:
NVM_HOME: "nvm"
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/

steps:
- uses: actions/checkout@v4

- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
- run: yarn install --frozen-lockfile

# Enable corepack
- name: Enable corepack
run: |
corepack enable

- run: yarn install --immutable

- run: yarn test
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "my-dev-tools",
"version": "4.0.0",
"version": "5.0.0-beta.0",
"main": "",
"repository": "https://github.com/begin-again/dev-tools.git",
"license": "MIT",
Expand All @@ -27,7 +27,7 @@
"chai": "^4.5.0",
"eslint": "^8.57.0",
"eslint-plugin-node": "^11.1.0",
"mocha": "^10.6.0",
"mocha": "^10.7.3",
"mock-fs": "^5.4.0",
"nyc": "^17.1.0",
"proxyquire": "^2.1.3",
Expand Down
2 changes: 1 addition & 1 deletion src/alias.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ alias ll='ls -ltrh'
## tools
alias resource='source $HOME/.bashrc'
alias clean='node $DEVROOT/dev-tools/src/clean/index.js'
alias grefp='node $DEVROOT/dev-tools/src/grefplus'
alias grefp='node $DEVROOT/dev-tools/src/grefplus/index.mjs'
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Incorrect path in grefp alias due to unset $DEVROOT.

  • Update the alias in src/alias.sh to: 
    alias grefp='node ./src/grefplus/index.mjs'
🔗 Analysis chain

LGTM! The alias update aligns with the PR objectives.

The change from grefp='node $DEVROOT/dev-tools/src/grefplus' to grefp='node $DEVROOT/dev-tools/src/grefplus/index.mjs' correctly reflects the transition to ES modules as mentioned in the PR objectives.

To ensure this change doesn't introduce breaking changes, please run the following verification:

This script will verify the existence of the new file, check for ES module syntax, and attempt to run the grefp command.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of the new grefplus index.mjs file and check its content

# Test 1: Check if the new file exists
if [ -f "$DEVROOT/dev-tools/src/grefplus/index.mjs" ]; then
    echo "index.mjs file exists."
else
    echo "Error: index.mjs file not found!"
    exit 1
fi

# Test 2: Check if the file contains ES module syntax
if grep -q "export" "$DEVROOT/dev-tools/src/grefplus/index.mjs"; then
    echo "index.mjs contains ES module export syntax."
else
    echo "Warning: index.mjs might not be using ES module syntax."
fi

# Test 3: Try running the grefp command with --help to check if it's working
if grefp --help > /dev/null 2>&1; then
    echo "grefp command executed successfully."
else
    echo "Error: grefp command failed to execute."
    exit 1
fi

Length of output: 130

Script:

#!/bin/bash
# Description: Output the value of the DEVROOT environment variable

echo "DEVROOT is set to: $DEVROOT"

Length of output: 48

Script:

#!/bin/bash
# Description: Search for the index.mjs file in the repository

fd "index.mjs" .

Length of output: 41

alias branches='node $DEVROOT/dev-tools/src/branches'
alias nt='node $DEVROOT/dev-tools/src/node-tools'
alias yn='node $DEVROOT/dev-tools/src/yarn'
Expand Down
110 changes: 60 additions & 50 deletions src/grefplus/cmdline.js → src/grefplus/cmdline.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { accessSync, constants, lstatSync } = require('fs');
const yargs = require('yargs');
const { DateTime } = require('luxon');
import { accessSync, constants, lstatSync } from 'node:fs';
import yargs from 'yargs/yargs';
import { DateTime } from 'luxon';

const options = {
dateOptions: 'yyyy-MM-dd hh:mm:ss a'
Expand Down Expand Up @@ -32,8 +32,7 @@ const cmdKeys = {
alias: 'r'
, describe: 'root folder of development environment (/c/blah/blah). Default is DEVROOT'
, type: 'array'
// eslint-disable-next-line no-process-env
, default: process.env.DEVROOT
, default: [process.env.DEVROOT]
}
, 'date': {
alias: 'd'
Expand All @@ -46,13 +45,14 @@ const cmdKeys = {
/**
* Validates from and to dates,
*
* @param {String} checkDate
* @param {string} checkDate
* @param {string} msg
* @throws if date is not valid
* @private
*/
const validateDate = (checkDate, msg) => {
try {
const parsed = DateTime.fromFormat(checkDate, options.allowedFormat);
const parsed = DateTime.fromFormat(checkDate.toString(), options.allowedFormat);
if(!parsed.isValid) {
throw new Error('unknown format');
}
Expand All @@ -69,11 +69,11 @@ const validateDate = (checkDate, msg) => {
/**
* Aborts build if dev root path does not exist
*
* @param {String} devRoot
* @param {string[]} devRoot
* @throws if path not accessible
* @private
*/
const validatePath = ({ devRoot }) => {
const validatePath = (devRoot) => {
let problematicRoot = null;

devRoot.forEach(root => {
Expand Down Expand Up @@ -108,24 +108,47 @@ const isFuture = (checkDate) => {
return _date > endOfToday;
};


/**
*
* @param {string} argName
* @param {boolean} [checkFuture]
* @returns {boolean}
* @throws if date is not valid
*/
const validateArgDate = (argName, checkFuture) => {
return (argv) => {
const dt = `${argv[argName]}` || '';
if(dt) {
validateDate(dt, `--${argName}`);
if(checkFuture && isFuture(dt)) {
throw new Error(`--${argName} cannot exceed current date`);
}
}
return true;
};
};

/**
* Parse command line and configures options
*
* @param {Boolean} test - used for testing only
* @param {Boolean} [test] - used for testing only
*/
const setOptions = (test) => {
const argv = test || yargs
const argv = test || yargs(process.argv.slice(2))
// @ts-ignore
.options(cmdKeys)
Comment on lines +138 to +139
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

LGTM with a minor correction needed

The updates to the setOptions function are good:

  1. The use of validateArgDate for date checks improves code maintainability.
  2. The folderNames check now correctly handles non-array cases.

However, there's a small issue to address:

There's a typo in one of the validateArgDate calls. The third call should be for 'toDate', not 'fromDate':

- .check(validateArgDate('fromDate', true))
+ .check(validateArgDate('toDate'))

Please correct this to ensure proper validation of the 'toDate' argument.

Also applies to: 164-178

.version(false)
.help(true)
.strict(true)
.check((_argv) => {
// super secret shortcut
if(!_argv.date && !_argv.fromDate && !_argv.toDate && Number.isInteger(_argv._[0])) {
const days = Number(_argv._[0]);
const _date = DateTime
.fromFormat(_argv._[0], options.allowedFormat)
.plus({ days: _argv._[0] })
.format('MM/DD/YY');
.fromFormat(days.toString(), options.allowedFormat)
.plus({ days })
.toFormat('MM/DD/YY');
_argv.date = _date;
Comment on lines +147 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrect date calculation in shortcut

The changes to the date shortcut calculation don't fully address the previous issue:

  1. Attempting to parse days.toString() as a date will fail because it's a number, not a valid date string.
  2. The final date formatting uses 'MM/DD/YY', which doesn't match the options.allowedFormat.

Here's a suggested fix:

const days = Number(_argv._[0]);
const _date = DateTime
    .now()
    .plus({ days })
    .toFormat(options.allowedFormat);
_argv.date = _date;

This change:

  1. Starts with the current date.
  2. Adds the specified number of days.
  3. Formats the result using the allowed format.

This approach ensures correct date calculation and consistent formatting.

}
Comment on lines +147 to 152
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Logical Error: Incorrect date calculation in the shortcut

The code attempts to parse days.toString() using options.allowedFormat, which is likely to fail since days is a number and may not match the date format. This results in an invalid date parsing.

Proposed fix:

Start with the current date and add days to it:

if(!_argv.date && !_argv.fromDate && !_argv.toDate && Number.isInteger(_argv._[0])) {
    const days = Number(_argv._[0]);
    const _date = DateTime
-       .fromFormat(days.toString(), options.allowedFormat)
+       .now()
        .plus({ days })
-       .toFormat('MM/DD/YY');
+       .toFormat(options.allowedFormat);
    _argv.date = _date;
}

This change initializes the date from the current date and adds the specified number of days, then formats it using the allowed format.

Committable suggestion was skipped due to low confidence.

else {
Expand All @@ -138,53 +161,40 @@ const setOptions = (test) => {
}
return true;
})
.check(({ date }) => {
if(date) {
validateDate(date, '--date');
if(isFuture(date)) {
throw new Error('--date cannot exceed current date');
}
}
return true;
})
.check(({ fromDate }) => {
if(fromDate) {
validateDate(fromDate, '--from-date');
if(isFuture(fromDate)) {
throw new Error('--from-date cannot exceed current date');
}
}
return true;
})
.check(({ toDate }) => {
if(toDate) {
validateDate(toDate, '--to-date');
}
return true;
})
.check(({ folderNames }) => {
.check(validateArgDate('date', true))
.check(validateArgDate('fromDate', true))
.check(validateArgDate('fromDate', true))
.check(validateArgDate('toDate'))
.check((argv) => {
const folderNames = Array.isArray(argv.folderNames) ? argv.folderNames : [];
if(folderNames && !folderNames.length) {
throw new Error('--folder-names requires at least one name');
}
return true;
})
.check(validatePath)
.check(argv => {
const devRoot = argv.devRoot;
return validatePath(devRoot);
})
.argv;

// @ts-ignore
if(argv.date) {
// @ts-ignore
const date = DateTime.fromFormat(argv.date, options.allowedFormat);
module.exports.options.fromDate = date.startOf('day');
module.exports.options.toDate = date.endOf('day');
options.fromDate = date.startOf('day');
options.toDate = date.endOf('day');
}
else {
module.exports.options.fromDate = argv.fromDate ? DateTime.fromFormat(argv.fromDate, options.allowedFormat).startOf('day') : null;
module.exports.options.toDate = argv.toDate ? DateTime.fromFormat(argv.toDate, options.allowedFormat).endOf('day') : null;
// @ts-ignore
options.fromDate = argv.fromDate ? DateTime.fromFormat(argv.fromDate, options.allowedFormat).startOf('day') : null;
// @ts-ignore
options.toDate = argv.toDate ? DateTime.fromFormat(argv.toDate, options.allowedFormat).endOf('day') : null;
}
module.exports.options.devRoot = argv.devRoot;
module.exports.options.folderNames = argv.folderNames || [];
// @ts-ignore
options.devRoot = argv.devRoot;
// @ts-ignore
options.folderNames = argv.folderNames || [];
};

module.exports = {
options
, setOptions
};
export { options, setOptions };
21 changes: 14 additions & 7 deletions src/grefplus/index.js → src/grefplus/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
/* eslint no-console:off */
require('./cmdline').setOptions();
const { basename } = require('path');
const { allRepoPaths } = require('../common/repos');
const { promisify } = require('util');
const _exec = promisify(require('child_process').exec);
const { DateTime } = require('luxon');
const { options } = require('./cmdline');

import { setOptions, options } from './cmdline.mjs';
import { basename } from 'node:path';
import { promisify } from 'node:util';
import { exec } from 'node:child_process';
const _exec = promisify(exec);

import { DateTime } from 'luxon';

import repos from '../common/repos.js';
const { allRepoPaths } = repos;

const DateLength = 6;

setOptions();

/**
* Creates command string
* @param {String} repo - repository base name
Expand Down
Loading
Loading