DX: Split out feature flags from the Me user data endpoint #7734
Conversation
Co-authored-by: Graham Langford <30706330+grahamlangford@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7734 +/- ##
==========================================
+ Coverage 72.14% 72.17% +0.03%
==========================================
Files 1270 1270
Lines 39760 39770 +10
Branches 7375 7371 -4
==========================================
+ Hits 28684 28705 +21
+ Misses 11076 11065 -11 ☔ View full report in Codecov by Sentry. |
|
No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack. Do not edit this comment manually. |
| }), | ||
| providesTags: ["Me"], | ||
| }), | ||
| getFeatureFlags: builder.query<string[], void>({ |
Axios already throws on HTTP errors
This comment has been minimized.
This comment has been minimized.
| await waitForEffect(); | ||
| // Make sure we only fetched once | ||
| expect(appApiMock.history.get).toHaveLength(1); |
There was a problem hiding this comment.
did await waitFor(() => { ... } not work for this?
There was a problem hiding this comment.
I had a case where it would eventually make two api calls, but using waitFor() it would run the assertion successfully at the point where only one call had been made and the test would pass, incorrectly.
| flags: ["test-flag"], | ||
| }); | ||
|
|
||
| const { result, waitFor } = renderHook(() => useFlags()); |
This comment has been minimized.
This comment has been minimized.
package-lock.json
Outdated
| @@ -235,7 +235,7 @@ | |||
| "css-minimizer-webpack-plugin": "^6.0.0", | |||
| "dotenv": "^16.4.5", | |||
| "eslint": "^8.57.0", | |||
| "eslint-config-pixiebrix": "^0.37.1", | |||
| "eslint-config-pixiebrix": "^0.36.2", | |||
There was a problem hiding this comment.
Why is this all getting messed up when I merge main into my branch? CC @fregante @grahamlangford
There was a problem hiding this comment.
Whats your exact sequence of actions when you merge the main branch and find conflicts? Do you use the CLI or also some UI for git?
There was a problem hiding this comment.
I start on my current dev branch, then I do:
git checkout main && git pull && git checkout <my_branch>
Then I run:
git merge main
When this runs, I see this npm-merge-driver: merging package-lock.json message in the output:
At this point, I need to run npm i again, otherwise if I push changes here then package-lock is messed up. Is this expected? Like, is it just npm changes on main and I need to run npm i after merging main as a normal step in the process?
There was a problem hiding this comment.
My standard workflow is just to run git pull origin main from my feature branch. Haven't had any problems with it.
There was a problem hiding this comment.
I'm curious if part of your problem is what ever changed between npm 10.2.5 and 10.3.0? Would need to look at the feature changes. But I doubt a minor would have that radical an effect.
There was a problem hiding this comment.
npm-merge-driver
You'll have to uninstall that, it's what's causing the issues. Back when I posted about it on Slack, I withdrew my suggestion a few minutes later.
There was a problem hiding this comment.
I need to run npm i after merging main as a normal step in the process?
This is only required if the lockfile has conflicts, because it will resolve them automatically (after you resolve any conflicts in package.json).
Even if there are no conflicts, it's possible that you still need to run the install to make sure you have the latest dependencies, if any were changed.
What does this PR do?
token.tstoauthStorage.tsto match other modules in this folder, and better describe what this module doesRemaining work
featureFlagsChecklist
src/tsconfig.strictNullChecks.json(if possible)