Skip to content
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

Chore: Package updates #49

Merged
merged 11 commits into from
Jul 7, 2023
Merged

Chore: Package updates #49

merged 11 commits into from
Jul 7, 2023

Conversation

chawes13
Copy link
Collaborator

@chawes13 chawes13 commented Jul 4, 2023

Resolves #45
Resolves #39 (except for redux-actions)

I didn't update redux-actions to v3 because they switched to ESM only, which our current Jest configuration doesn't understand. I honestly can't be fussed to dig into it further at the minute given how much of a headache ESM/CJS has become.

@@ -12,8 +12,7 @@
"prepublish": "yarn run lint && yarn run clean && yarn run build",
"test": "jest",
"size": "yarn build && size-limit",
"test:coverage": "jest --coverage",
"report-coverage": "codeclimate-test-reporter < coverage/lcov.info"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This command doesn't work on main currently and it might never have worked. The package itself has been deprecated. Given that it's not used by anything, I'd rather remove it then "fix" it.

Comment on lines +35 to +36
"@size-limit/file": "^8.2.6",
"@size-limit/webpack": "^8.2.6",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@size-limit switched from defaulting to webpack in v6 to esbuild in v7/v8. For some reason, this caused our reported build size to nearly quadruple (8.63 kb -> 31.05 kb). I tried to dig into what was going on, but I wasn't able to make much progress.

Given that webpack is what we were using to determine the size, this feels like less of a breaking change (despite the swap out in packages) and intuitively makes more sense.

@@ -21,31 +21,31 @@ test('flashMessage sets default timeout if none is provided', () => {
const store = createMockStore()
store.dispatch(flashMessage('Hi'))
expect(store.getActionTypes()).toEqual([ADD_MESSAGE_ACTION_TYPE])
jest.runTimersToTime(4000)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This method was deprecated

@@ -5,6 +5,8 @@ import {
} from '../src'
import { createMockStore } from './helpers'

jest.useFakeTimers()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this, jest was reporting a potential unhandled operation (as a result of setTimeout in middleware.js)

Copy link

Choose a reason for hiding this comment

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

Wow. I'm sure that was fun to figure out.

@chawes13 chawes13 requested a review from mwislek July 4, 2023 17:17
Copy link

@mwislek mwislek left a comment

Choose a reason for hiding this comment

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

Looks great Conor!

@chawes13 chawes13 merged commit ff9755b into main Jul 7, 2023
@chawes13 chawes13 deleted the 2023-07-package-updates branch July 7, 2023 01:27
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.

Dependency Dashboard Update uuid to version 7 or higher
2 participants