Skip to content

@uppy: upgrade biome and more improvements#6244

Merged
mifi merged 13 commits into
transloadit:mainfrom
qxprakash:upgrade_biome
May 21, 2026
Merged

@uppy: upgrade biome and more improvements#6244
mifi merged 13 commits into
transloadit:mainfrom
qxprakash:upgrade_biome

Conversation

@qxprakash
Copy link
Copy Markdown
Collaborator

@qxprakash qxprakash commented Mar 26, 2026

  • This PR upgrades biome from 2.0.5 -> 2.1.2 and adds two new rules
  • remove stale suppressions.
  • remove stale code.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 26, 2026

⚠️ No Changeset found

Latest commit: ebbfbc9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dde27afd84

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/@uppy/transloadit/src/AssemblyWatcher.ts
@qxprakash qxprakash requested a review from mifi March 26, 2026 12:27
@mifi
Copy link
Copy Markdown
Contributor

mifi commented Mar 26, 2026

looks good but I think we should wait until typescript pr #6179 is merged

@qxprakash qxprakash requested a review from Murderlon March 26, 2026 17:58
@mifi
Copy link
Copy Markdown
Contributor

mifi commented May 9, 2026

once the conflicts are resolved i think this can be merged

Comment thread biome.json Outdated
@qxprakash qxprakash marked this pull request as draft May 13, 2026 10:51
@qxprakash qxprakash self-assigned this May 13, 2026
@qxprakash qxprakash marked this pull request as ready for review May 13, 2026 11:20
Copy link
Copy Markdown
Collaborator Author

@qxprakash qxprakash left a comment

Choose a reason for hiding this comment

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

  • upgraded biome 2.1.2 → 2.4.15
  • added html.experimentalFullSupportEnabled: true
  • Moved noNestedComponentDefinitions + noReactPropAssign (renamed → noReactPropAssignments) from nursery to correctness (where they now live in 2.4)
  • added fixes triggered by Vue/Svelte/HTML now being properly linted
  • Cleanup forced by Biome 2.4 + TypeScript stricter checks

Leveraged AI heavily in making these changes.

@mifi
Copy link
Copy Markdown
Contributor

mifi commented May 18, 2026

lgtm but verify that the changess in biome.json and correct, and verify that code inside packages is still being linted after this change (packages among other things was removed from include)

Comment thread biome.json
Comment on lines 28 to -41
"linter": {
"enabled": true,
"includes": [
"packages/**",
"examples/**",
"scripts/**",
"private/**",
"!packages/**/{dist,lib}/**",
"!node_modules",
"!.svelte-kit",
"!packages/@uppy/components/src/input.css",
"!**/*.vue",
"!**/*.svelte"
],
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we can safely remove this now because biome 2.4 can fully lint .vue and .svelte files using the flag experimentalFullSupportEnabled: true , linter.includes falls back to files.includes , which already contains all the paths which should be linted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

previously we needed this block so that we can disable lint for .vue and svelte files. check the last two paths

      "!**/*.vue",
      "!**/*.svelte"

Comment thread biome.json
Comment on lines +77 to +112
"overrides": [
{
"includes": ["packages/@uppy/companion/**"],
"linter": {
"rules": {
"complexity": {
"useLiteralKeys": "off"
}
}
}
},
{
"includes": ["**/*.vue", "**/*.svelte", "packages/@uppy/vue/**/*.ts"],
"linter": {
"rules": {
"correctness": {
"useHookAtTopLevel": "off",
"useExhaustiveDependencies": "off"
}
}
}
},
{
"includes": ["examples/angular/**"],
"linter": {
"enabled": false
}
},
{
"includes": ["examples/nextjs/**/*.css"],
"css": {
"parser": {
"tailwindDirectives": true
}
}
}
Copy link
Copy Markdown
Collaborator Author

@qxprakash qxprakash May 18, 2026

Choose a reason for hiding this comment

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

this override block is used for overriding lint rules for certain paths , I will check them to see what do we actually need , for example : the first rule https://biomejs.dev/linter/rules/use-literal-keys/
disables the enforcement of literal access of objects i.e. obj.someKey instead of compuated access i.e. obj["someKey"] because companion uses it a lot , so rules like these must have been added by AI to reduce the linter noise.

    {
      "includes": ["packages/@uppy/companion/**"],
      "linter": {
        "rules": {
          "complexity": {
            "useLiteralKeys": "off"
          }
        }

@qxprakash
Copy link
Copy Markdown
Collaborator Author

lgtm but verify that the changess in biome.json and correct, and verify that code inside packages is still being linted after this change (packages among other things was removed from include)

verified ! , linting works in a random uppy plugin and random example. biome changes look correct still I will add small summaries of the reasoning given by AI and then we can decide on what to and what to revert.

@socket-security
Copy link
Copy Markdown

socket-security Bot commented May 18, 2026

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

Comment thread biome.json
Comment on lines +79 to +87
"includes": ["packages/@uppy/companion/**"],
"linter": {
"rules": {
"complexity": {
"useLiteralKeys": "off"
}
}
}
},
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is added due to a conflict : companion's tsconfig.shared.json sets noPropertyAccessFromIndexSignature: true. This is a deliberate, strict TypeScript option that says:

"If a property comes from an index signature (like process.env, req.query, req.body), you MUST use
bracket notation to access it."

Comment thread biome.json
Comment on lines +89 to +98
"includes": ["**/*.vue", "**/*.svelte", "packages/@uppy/vue/**/*.ts"],
"linter": {
"rules": {
"correctness": {
"useHookAtTopLevel": "off",
"useExhaustiveDependencies": "off"
}
}
}
},
Copy link
Copy Markdown
Collaborator Author

@qxprakash qxprakash May 18, 2026

Choose a reason for hiding this comment

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

AI Summary :

What policy this protects: the framework's own reactivity model

These two rules are React-specific:

  • useHookAtTopLevel - React's "Rules of Hooks" (no conditionals, no loops, top-level only)
  • useExhaustiveDependencies — React's useEffect / useMemo / useCallback dependency-array discipline

Biome triggers them on any identifier starting with use*. But:

  • Vue composables (useUppy, useDropzone, useFileInput) have no rules-of-hooks constraint — they're
    plain functions returning reactive refs. They can be called conditionally, in loops, anywhere.
  • Svelte 5 runes ($state, $derived, $effect) have automatic dependency tracking — there is no manual
    dependency array to keep exhaustive.

Comment thread biome.json Outdated
Comment on lines +100 to +103
"includes": ["examples/angular/**"],
"linter": {
"enabled": false
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I tested without this override , it worked fine so I think we can remove this.

Comment thread biome.json
Comment on lines +106 to +111
"includes": ["examples/nextjs/**/*.css"],
"css": {
"parser": {
"tailwindDirectives": true
}
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

AI summary :

The Next.js example uses Tailwind v4 directives:

 @tailwind base;
  @tailwind components;
  @tailwind utilities;
  @apply text-lg font-bold;

These are not standard CSS. Without the tailwindDirectives: true setting, Biome's CSS parser errors
on @tailwind and @apply as unknown at-rules. Then the file fails to parse → nothing can be linted or
formatted.

@mifi
Copy link
Copy Markdown
Contributor

mifi commented May 20, 2026

let's merge this after #6300 is merged - there might be some conflicts in yarn.lock

@mifi mifi mentioned this pull request May 20, 2026
9 tasks
@qxprakash qxprakash mentioned this pull request May 20, 2026
mifi added a commit that referenced this pull request May 21, 2026
exact copy of #6175 

- I will point the uppy deps upgrade pr to this PR's branch
- no other changes needed in this branch as #6175 was reviewed and all
the comments were resolved

this should be the order of merge : 

1. #6300
2. uppy deps upgrade ( yet to be raised ) 
3. merge this to main
4. merge biome changes #6244 

@mifi let me know if you think otherwise.

---------

Co-authored-by: Mikael Finstad <finstaden@gmail.com>
@mifi mifi merged commit 5519b84 into transloadit:main May 21, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants