Skip to content

Comments

[Site-Admin]: Add history@5.3.0 dependency (ARC-31)#102067

Merged
retrofox merged 2 commits intotrunkfrom
update/site-admin-add-history-package-dep
Mar 31, 2025
Merged

[Site-Admin]: Add history@5.3.0 dependency (ARC-31)#102067
retrofox merged 2 commits intotrunkfrom
update/site-admin-add-history-package-dep

Conversation

@retrofox
Copy link
Contributor

This PR adds the history package dependency to the Site Admin package.

Related to #

Proposed Changes

  • [Site-Admin]: Add history@5.3.0 dependency

Why are these changes being made?

  • [Site-Admin]: Add history@5.3.0 dependency

Testing Instructions

A visual review should be enough.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@retrofox retrofox self-assigned this Mar 28, 2025
@github-actions
Copy link

github-actions bot commented Mar 28, 2025

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • blaze-dashboard
  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/site-admin-add-history-package-dep on your sandbox.

@retrofox retrofox requested a review from psealock March 31, 2025 07:10
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 31, 2025
@retrofox retrofox requested a review from a team March 31, 2025 07:10
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

The changes individually look good 👍

Might make sense to see how the package will be used, specifically with types, since I can see we still use an old version of @types/history as a dependency somewhere. With that in mind this could be part of a larger PR.

@retrofox retrofox merged commit 5031ee2 into trunk Mar 31, 2025
15 checks passed
@retrofox retrofox deleted the update/site-admin-add-history-package-dep branch March 31, 2025 10:53
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 31, 2025
@retrofox
Copy link
Contributor Author

Might make sense to see how the package will be used, specifically with types, since I can see we still use an old version of @types/history as a dependency somewhere. With that in mind this could be part of a larger PR.

It seems that we don't need @types/history package anymore:

This is a stub types definition. history provides its own type definitions, so you do not need this installed.

npm package

@tyxla
Copy link
Member

tyxla commented Mar 31, 2025

That's correct - we don't need it, which is why we should keep an eye on where and how it's being used

Copy link
Contributor Author

Here is where it is used. It doesn't look pretty well, btw 😀

@tyxla
Copy link
Member

tyxla commented Mar 31, 2025

Here is where it is used. It doesn't look pretty well, btw 😀

Right. Would be nice if we could remove that hack.

@ciampo
Copy link
Contributor

ciampo commented Apr 2, 2025

Here is where it is used. It doesn't look pretty well, btw 😀

Right. Would be nice if we could remove that hack.

On it #102340

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