Skip to content

Remove provider-constants-slice.ts in favor of direct constant imports#2759

Merged
bcotrim merged 4 commits intotrunkfrom
stu-1378-remove-provider-constants-slice
Mar 13, 2026
Merged

Remove provider-constants-slice.ts in favor of direct constant imports#2759
bcotrim merged 4 commits intotrunkfrom
stu-1378-remove-provider-constants-slice

Conversation

@bcotrim
Copy link
Copy Markdown
Contributor

@bcotrim bcotrim commented Mar 11, 2026

Related issues

How AI was used in this PR

AI was used to generate the initial version, some manual adjustments to remove unnecessary type casts.

Proposed Changes

  • Delete the Redux slice provider-constants-slice.ts which stored static PHP and WordPress version constants as Redux state
  • Replace all Redux selectors with direct constant imports from @studio/common/constants and @studio/common/types/php-versions
  • Updated 6 files to use constants directly instead of Redux state
  • Removed unnecessary local variable redeclarations and type casts
  • Removed test setup code that was dispatching Redux actions for static constants

Testing Instructions

  • TypeScript type checker passes
  • All 5 tests in use-add-site.test.tsx pass
  • Linter passes with no warnings

Pre-merge Checklist

  • TypeScript errors checked

@bcotrim bcotrim self-assigned this Mar 11, 2026
@wpmobilebot
Copy link
Copy Markdown
Collaborator

wpmobilebot commented Mar 11, 2026

📊 Performance Test Results

Comparing d9878ee vs trunk

app-size

Metric trunk d9878ee Diff Change
App Size (Mac) 1278.62 MB 1278.61 MB 0.01 MB ⚪ 0.0%

site-editor

Metric trunk d9878ee Diff Change
load 1889 ms 1919 ms +30 ms ⚪ 0.0%

site-startup

Metric trunk d9878ee Diff Change
siteCreation 6086 ms 6124 ms +38 ms ⚪ 0.0%
siteStartup 3942 ms 3941 ms 1 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

@bcotrim bcotrim requested a review from a team March 11, 2026 15:37
Copy link
Copy Markdown
Contributor

@fredrikekelund fredrikekelund left a comment

Choose a reason for hiding this comment

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

Looking good overall, but I left one comment that would be good to address before landing

Comment thread apps/studio/src/hooks/use-add-site.ts Outdated
Comment on lines +329 to +330
defaultPhpVersion: DEFAULT_PHP_VERSION,
defaultWpVersion: DEFAULT_WORDPRESS_VERSION,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAICT, these two props are only consumed in apps/studio/src/modules/add-site/index.tsx. Could we remove them from this file and just import DEFAULT_PHP_VERSION and DEFAULT_WORDPRESS_VERSION directly in that file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Copy Markdown
Contributor

@katinthehatsite katinthehatsite left a comment

Choose a reason for hiding this comment

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

I think the changes look good. I did not see any regressions and I think that this is a reasonable optimization 👍

@bcotrim bcotrim merged commit 8265d6b into trunk Mar 13, 2026
10 checks passed
@bcotrim bcotrim deleted the stu-1378-remove-provider-constants-slice branch March 13, 2026 17:50
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.

4 participants