Skip to content

guardian/eslint-config-typescript v11#14659

Merged
JamieB-gu merged 3 commits intomainfrom
guardian-eslint-config-typescript-v11
Oct 14, 2025
Merged

guardian/eslint-config-typescript v11#14659
JamieB-gu merged 3 commits intomainfrom
guardian-eslint-config-typescript-v11

Conversation

@JamieB-gu
Copy link
Contributor

@JamieB-gu JamieB-gu commented Oct 9, 2025

The supported TypeScript version is now 5.5.2. Running --fix resulted in two changes, which involve replacing expressions like this:

foo && foo.includes(...) ? true : false

with:

foo?.includes(...) ? true : false

This also fixes a warning about an unsupported TypeScript version when running ESLint.

Release notes: https://github.com/guardian/csnx/releases/tag/%40guardian%2Feslint-config-typescript%4011.0.0

Note: This is opened against #14658 until that's merged.

It now requires curly braces, which have been added where needed in this
change using `--fix`. This also resulted in some small changes to how
types are defined in a few places.
The supported TypeScript version is now 5.5.2. Running `--fix` resulted
in one change, replacing the expression:

```
foo && foo.includes(...) ? true : false
```

with:

```
foo?.includes(...) ? true : false
```

This also fixes a warning about an unsupported TypeScript version when
running ESLint.
@JamieB-gu JamieB-gu added this to the Health milestone Oct 9, 2025
@JamieB-gu JamieB-gu self-assigned this Oct 9, 2025
@JamieB-gu JamieB-gu requested a review from a team as a code owner October 9, 2025 16:19
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

@JamieB-gu JamieB-gu marked this pull request as draft October 9, 2025 16:20
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Base automatically changed from guardian-eslint-config-typescript-v10 to main October 13, 2025 12:23
Comment on lines +331 to +335
const isDatawrapperGraphic = url?.includes(
'interactive.guim.co.uk/datawrapper',
)
? true
: false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the result of the expression should be the same. Technically the behaviour changes slightly in the empty string ('') case. The previous behaviour:

  1. '' is falsy
  2. '' && ''.includes(...) evaluates to '', skipping the includes
  3. That's coerced to false
  4. The ternary condition is false, so it returns false

Now:

  1. '' is neither undefined or null
  2. Optional chaining doesn't short circuit, so includes is evaluated
  3. '' doesn't include the string being tested for, so the result is false
  4. The ternary condition is false, so it returns false

As in practice the empty string will not include either the string being tested here, or the one in the scriptUrlIsBoot expression below, there should be no change in functionality.

Does that sound correct @RikRootsGuardian ? I believe it was your change that introduced these expressions originally?

@JamieB-gu JamieB-gu added the run_chromatic Runs chromatic when label is applied label Oct 13, 2025
@JamieB-gu JamieB-gu marked this pull request as ready for review October 13, 2025 12:43
@github-actions github-actions bot removed the run_chromatic Runs chromatic when label is applied label Oct 13, 2025
@JamieB-gu JamieB-gu merged commit 07e7546 into main Oct 14, 2025
33 checks passed
@JamieB-gu JamieB-gu deleted the guardian-eslint-config-typescript-v11 branch October 14, 2025 10:20
@gu-prout
Copy link

gu-prout bot commented Oct 14, 2025

Seen on PROD (merged by @JamieB-gu 8 minutes ago) Please check your changes!

@JamieB-gu JamieB-gu added the maintenance Departmental tracking: maintenance work, not a fix or a feature label Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

apps-rendering dotcom-rendering maintenance Departmental tracking: maintenance work, not a fix or a feature Seen-on-PROD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants