-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Update history package to the latest 5.3.0 version #33027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
chihsuan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@moon0326 Looks good to me. But it seems that one unit test is failing due to the package update.
Summary of all failing tests
FAIL navigation/components/container/test/index.js
● Container › should update the active item and level when location changes
TypeError: window.location.assign is not a function
40 | },
41 | createHref: (...args) => browserHistory.createHref.apply(browserHistory, args),
> 42 | push: (...args) => browserHistory.push.apply(browserHistory, args),
| ^
43 | replace: (...args) => browserHistory.replace.apply(browserHistory, args),
44 | go: (...args) => browserHistory.go.apply(browserHistory, args),
45 | goBack: (...args) => browserHistory.goBack.apply(browserHistory, args),
at Object.push (../../../node_modules/.pnpm/[email protected]/node_modules/history/umd/history.development.js:240:27)
at Object.push (../../../packages/js/navigation/build/history.js:42:52)
at navigation/components/container/test/index.js:157:17
at batchedUpdates$1 (../../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom.development.js:22380:12)
at act (../../../node_modules/.pnpm/[email protected][email protected]/node_modules/react-dom/cjs/react-dom-test-utils.development.js:1042:14)
at Object.<anonymous> (navigation/components/container/test/index.js:152:12)
Test Suites: 1 failed, 85 passed, 86 total
Tests: 1 failed, 577 passed, 578 total
Snapshots: 20 passed, 20 total
Time: 31.784 s
9d21a53 to
29fcbfa
Compare
|
📊 Test reports for this pull request have been published and are accessible through the following links:
Latest commit referenced in the reports: Visit the WooCommerce Test Reports homepage to view all published reports. See the FAQs page if you're having problems accessing them. |
Thank you for catching it! Fixed in 29fcbfa |
chihsuan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM🚀
|
Hi @moon0326, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:
|
All Submissions:
Changes proposed in this Pull Request:
Closes #32881 .
This PR updates
historypackage to the latest5.3.0It also removes@types/historyas it is part ofhisotrynow.Please keep in mind that we're updating to a major version with the following breaking changes. We're not affected from the braking changes as far as I know.
How to test the changes in this Pull Request:
There are no visual changes.
WooCommerce -> Settings -> Advanced -> Featuresand make sure the Navigation works as expected without any errors.Other information:
pnpm nx changelog <project>?FOR PR REVIEWER ONLY: