From 7ee159bfead398b4ae1bc76b7961752bb0e35ac5 Mon Sep 17 00:00:00 2001 From: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com> Date: Fri, 6 Sep 2024 19:55:30 +0100 Subject: [PATCH 1/3] fix(linting): Re-enable babel presets during linting of javascript projects (#11458) Fixes #11457 In #10867 I removed the presets by default and they are only included in specific cases - for jest, for prerender. It looks like I forgot to consider the case of linting javascript projects. I therefore re-enable these presets in that specific case. It's already on my medium term wish list to simplify and rip out as much of our babel config/reliance as we can. It would be better if this was all just simpler and didn't have as many branches. I also introduced a copy of `isTypeScriptProject` into `@redwoodjs/project-config`. It doesn't add any dependencies to that package and it relies on the functionality already provided in it. I will make a note to refactor our existing usage to use this one. I think that would be better. --- .changesets/11458.md | 3 +++ packages/babel-config/src/web.ts | 3 ++- packages/eslint-config/index.js | 7 +++++-- packages/project-config/src/paths.ts | 8 ++++++++ 4 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 .changesets/11458.md diff --git a/.changesets/11458.md b/.changesets/11458.md new file mode 100644 index 000000000000..710caf38aca2 --- /dev/null +++ b/.changesets/11458.md @@ -0,0 +1,3 @@ +- fix(linting): Re-enable babel presets during linting of javascript projects (#11458) by @Josh-Walker-GM + +The `yarn rw lint` command was failing for JavaScript projects. This change re-enables certain babel plugins to correct this issue and allow this command to succeed again. diff --git a/packages/babel-config/src/web.ts b/packages/babel-config/src/web.ts index 77ee8c364345..24c852f5d836 100644 --- a/packages/babel-config/src/web.ts +++ b/packages/babel-config/src/web.ts @@ -20,6 +20,7 @@ export interface Flags { forJest?: boolean // will change the alias for module-resolver plugin forPrerender?: boolean // changes what babel-plugin-redwood-routes-auto-loader does forRsc?: boolean + forJavaScriptLinting?: boolean // will enable presets to supporting linting in the absence of typescript related presets/plugins/parsers } export const getWebSideBabelPlugins = ( @@ -166,7 +167,7 @@ export const getWebSideOverrides = ( export const getWebSideBabelPresets = (options: Flags) => { // When we perform prerendering we don't use vite, so we need to add the // appropriate presets for react, env, and typescript, etc. - if (options.forPrerender || options.forJest) { + if (options.forPrerender || options.forJest || options.forJavaScriptLinting) { let reactPresetConfig: babel.PluginItem = { runtime: 'automatic' } // This is a special case, where @babel/preset-react needs config diff --git a/packages/eslint-config/index.js b/packages/eslint-config/index.js index 20ef7945d450..06cced2c1292 100644 --- a/packages/eslint-config/index.js +++ b/packages/eslint-config/index.js @@ -7,7 +7,7 @@ const { getApiSideDefaultBabelConfig, getWebSideDefaultBabelConfig, } = require('@redwoodjs/babel-config') -const { getConfig } = require('@redwoodjs/project-config') +const { getConfig, isTypeScriptProject } = require('@redwoodjs/project-config') const config = getConfig() @@ -16,7 +16,10 @@ const getProjectBabelOptions = () => { // So we just take it out and put it as a separate item // Ignoring overrides, as I don't think it has any impact on linting const { overrides: _webOverrides, ...otherWebConfig } = - getWebSideDefaultBabelConfig() + getWebSideDefaultBabelConfig({ + // We have to enable certain presets like `@babel/preset-react` for JavaScript projects + forJavaScriptLinting: !isTypeScriptProject(), + }) const { overrides: _apiOverrides, ...otherApiConfig } = getApiSideDefaultBabelConfig() diff --git a/packages/project-config/src/paths.ts b/packages/project-config/src/paths.ts index 3fe2b8b89b1d..b375abfa1acd 100644 --- a/packages/project-config/src/paths.ts +++ b/packages/project-config/src/paths.ts @@ -440,3 +440,11 @@ export function projectIsEsm() { return true } + +export const isTypeScriptProject = () => { + const paths = getPaths() + return ( + fs.existsSync(path.join(paths.web.base, 'tsconfig.json')) || + fs.existsSync(path.join(paths.api.base, 'tsconfig.json')) + ) +} From 1f81c6ef00e0d9e3c40f53db259f46bb3a56bbde Mon Sep 17 00:00:00 2001 From: Tobbe Lundberg Date: Fri, 6 Sep 2024 21:29:47 +0200 Subject: [PATCH 2/3] fix(jobs): Make deleteSuccessfulJobs configurable (#11459) --- .changesets/11459.md | 3 ++ packages/jobs/src/core/Executor.ts | 18 +++++----- .../jobs/src/core/__tests__/Executor.test.ts | 33 +++++++++++++++++++ 3 files changed, 45 insertions(+), 9 deletions(-) create mode 100644 .changesets/11459.md diff --git a/.changesets/11459.md b/.changesets/11459.md new file mode 100644 index 000000000000..8fb7232d291d --- /dev/null +++ b/.changesets/11459.md @@ -0,0 +1,3 @@ +- fix(jobs): Make deleteSuccessfulJobs configurable (#11459) by @Tobbe + +Make the jobs Executor respect the `deleteSuccessfulJobs` config option diff --git a/packages/jobs/src/core/Executor.ts b/packages/jobs/src/core/Executor.ts index 51ae7bc040c2..dad194d1df19 100644 --- a/packages/jobs/src/core/Executor.ts +++ b/packages/jobs/src/core/Executor.ts @@ -11,7 +11,7 @@ import { AdapterRequiredError, JobRequiredError } from '../errors.js' import { loadJob } from '../loaders.js' import type { BaseJob, BasicLogger } from '../types.js' -interface Options { +export interface ExecutorOptions { adapter: BaseAdapter job: BaseJob logger?: BasicLogger @@ -28,15 +28,15 @@ export const DEFAULTS = { } export class Executor { - options: Required - adapter: Options['adapter'] - logger: NonNullable + options: Required + adapter: ExecutorOptions['adapter'] + logger: NonNullable job: BaseJob - maxAttempts: NonNullable - deleteFailedJobs: NonNullable - deleteSuccessfulJobs: NonNullable + maxAttempts: NonNullable + deleteFailedJobs: NonNullable + deleteSuccessfulJobs: NonNullable - constructor(options: Options) { + constructor(options: ExecutorOptions) { this.options = { ...DEFAULTS, ...options } // validate that everything we need is available @@ -68,7 +68,7 @@ export class Executor { await this.adapter.success({ job: this.job, - deleteJob: DEFAULT_DELETE_SUCCESSFUL_JOBS, + deleteJob: this.deleteSuccessfulJobs, }) } catch (error: any) { this.logger.error( diff --git a/packages/jobs/src/core/__tests__/Executor.test.ts b/packages/jobs/src/core/__tests__/Executor.test.ts index 83dbd4162214..fc5f36d5137c 100644 --- a/packages/jobs/src/core/__tests__/Executor.test.ts +++ b/packages/jobs/src/core/__tests__/Executor.test.ts @@ -4,6 +4,7 @@ import { DEFAULT_LOGGER } from '../../consts.js' import * as errors from '../../errors.js' import type { BaseJob } from '../../types.js' import { Executor } from '../Executor.js' +import type { ExecutorOptions } from '../Executor.js' import { MockAdapter, mockLogger } from './mocks.js' @@ -146,6 +147,38 @@ describe('perform', () => { }) }) + it('keeps the job around after successful job if instructed to do so', async () => { + const mockAdapter = new MockAdapter() + const mockJob = { + id: 1, + name: 'TestJob', + path: 'TestJob/TestJob', + args: ['foo'], + attempts: 0, + + perform: vi.fn(), + } + const options: ExecutorOptions = { + adapter: mockAdapter, + logger: mockLogger, + job: mockJob, + deleteSuccessfulJobs: false, + } + const executor = new Executor(options) + + // spy on the success function of the adapter + const adapterSpy = vi.spyOn(mockAdapter, 'success') + // mock the `loadJob` loader to return the job mock + mocks.loadJob.mockImplementation(() => mockJob) + + await executor.perform() + + expect(adapterSpy).toHaveBeenCalledWith({ + job: options.job, + deleteJob: false, + }) + }) + it('invokes the `failure` method on the adapter when job fails', async () => { const mockAdapter = new MockAdapter() const mockError = new Error('mock error in the job perform method') From 1cc6d0a63099c2b2466c660a05d1ce4d65a5d959 Mon Sep 17 00:00:00 2001 From: Josh GM Walker <56300765+Josh-Walker-GM@users.noreply.github.com> Date: Fri, 6 Sep 2024 21:16:29 +0100 Subject: [PATCH 3/3] chore(ci): fix type tests running in ci (#11460) We expect the root package json to run tests via nx run-many. If we just use tstyche then it won't pick up any custom behaviour that the individual test:types scripts define. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c3b068668405..4f6ad68ec032 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "test": "nx run-many -t test -- --minWorkers=1 --maxWorkers=4", "test-ci": "nx run-many -t test", "test:k6": "tsx ./tasks/k6-test/run-k6-tests.mts", - "test:types": "tstyche" + "test:types": "nx run-many -t test:types" }, "resolutions": { "@storybook/react-dom-shim@npm:7.6.17": "https://verdaccio.tobbe.dev/@storybook/react-dom-shim/-/react-dom-shim-8.0.8.tgz",