Skip to content
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.

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

Repo: Add CI job testing that yarn generate:configs still works #5925

Closed
JoshuaKGoldberg opened this issue Nov 3, 2022 · 3 comments · Fixed by #7418
Closed

Repo: Add CI job testing that yarn generate:configs still works #5925

JoshuaKGoldberg opened this issue Nov 3, 2022 · 3 comments · Fixed by #7418
Labels
accepting prs Go ahead, send a pull request that resolves this issue repo maintenance things to do with maintenance of the repo, and not with code/docs

Comments

@JoshuaKGoldberg
Copy link
Member

Suggestion

#5251 (comment): yarn generate:configs doesn't actually... work right now locally.

$ ../../node_modules/.bin/ts-node --files --transpile-only tools/generate-configs.ts
/home/josh/repos/typescript-eslint/packages/eslint-plugin/tools/generate-configs.ts:6
import rules from '../src/rules';
                                ^
Error [ERR_REQUIRE_ESM]: require() of ES Module /home/josh/repos/typescript-eslint/packages/eslint-plugin/node_modules/chalk/source/index.js from /home/josh/repos/typescript-eslint/packages/eslint-plugin/tools/generate-configs.ts not supported.
Instead change the require of index.js in /home/josh/repos/typescript-eslint/packages/eslint-plugin/tools/generate-configs.ts to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/home/josh/repos/typescript-eslint/packages/eslint-plugin/tools/generate-configs.ts:6:33)
    at Module.m._compile (/home/josh/repos/typescript-eslint/node_modules/ts-node/dist/index.js:791:29)
    at Object.require.extensions.<computed> [as .ts] (/home/josh/repos/typescript-eslint/node_modules/ts-node/dist/index.js:793:16)
    at phase4 (/home/josh/repos/typescript-eslint/node_modules/ts-node/dist/bin.js:407:16)
    at bootstrap (/home/josh/repos/typescript-eslint/node_modules/ts-node/dist/bin.js:49:12)
    at main (/home/josh/repos/typescript-eslint/node_modules/ts-node/dist/bin.js:32:12)
    at Object.<anonymous> (/home/josh/repos/typescript-eslint/node_modules/ts-node/dist/bin.js:519:5) {
  code: 'ERR_REQUIRE_ESM'
}

#4952 bumped chalk to v5, which is an ESM-only version. You can read about the context about ESM-only packages here: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c.

Unfortunately we're not yet on pure ESM for our source scripts, and so cannot use this new version of chalk. We'll have to downgrade chalk to v4.

We don't have any CI task verifying that yarn generate:configs does not crash - so we have no way of knowing that any change breaks that particular script, unless we think to explicitly test it.

Proposal: let's add a new job to ci.yml that verifies yarn generate:configs does not exit with an error code?

@JoshuaKGoldberg JoshuaKGoldberg added triage Waiting for maintainers to take a look repo maintenance things to do with maintenance of the repo, and not with code/docs labels Nov 3, 2022
@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for maintainers to take a look labels Nov 16, 2022
@bmish
Copy link
Contributor

bmish commented Nov 16, 2022

You could have a script like:

"generate": "npm-run-all \"generate:*\"",

And run it on CI to ensure all the generate scripts exit with success, just like all the linters.

@bradzacher
Copy link
Member

The CI chek should also assert that the generation scripts don't leave any changes in the code so we can validate the user has run the script and checked in the changes.

@bmish
Copy link
Contributor

bmish commented Nov 16, 2022

Either the scripts could set their exit codes based on that, or you could use && git diff --exit-code or && git status --porcelain to ensure there was no change outputted.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants