Skip to content

Commit 486e4cd

Browse files
authored
move useVideo to v2 deps (#1793)
## Summary: Experimenting porting v1 Perseus dependencies to v2 Perseus dependencies. Starting with `useVideo` because its use isn't very widespread. ## Test plan: Nothing should change, just moving where we add `useVideo` Author: handeyeco Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1793
1 parent 22d1c02 commit 486e4cd

File tree

9 files changed

+75
-42
lines changed

9 files changed

+75
-42
lines changed

.changeset/fuzzy-teachers-scream.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@khanacademy/perseus": major
3+
"@khanacademy/perseus-editor": patch
4+
---
5+
6+
Move useVideo from v1 dependency to v2 dependency

packages/perseus-editor/src/multirenderer-editor.tsx

+3-3
Original file line numberDiff line numberDiff line change
@@ -867,11 +867,11 @@ class MultiRendererEditor extends React.Component<MultiRendererEditorProps> {
867867
item={item}
868868
shape={itemShape}
869869
apiOptions={apiOptions}
870-
// Today, with analytics being the only thing in
871-
// dependencies, we send in a dummy function as we don't
872-
// want to gather analytics events from within the editor.
870+
// MultiRenderer is on its way out,
871+
// so I think it's save to use these dummy deps
873872
dependencies={{
874873
analytics: {onAnalyticsEvent: async () => {}},
874+
useVideo: (() => {}) as any,
875875
}}
876876
>
877877
{({renderers}) => (

packages/perseus/src/__tests__/widget-container.test.tsx

+9-3
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import {render, screen} from "@testing-library/react";
22
import * as React from "react";
33

4-
import {testDependencies} from "../../../../testing/test-dependencies";
4+
import {
5+
testDependencies,
6+
testDependenciesV2,
7+
} from "../../../../testing/test-dependencies";
58
import * as Dependencies from "../dependencies";
69
import WidgetContainer from "../widget-container";
710
import {registerWidget} from "../widgets";
811
import PassageWidget from "../widgets/passage";
912

10-
import type {WidgetExports} from "../types";
13+
import type {PerseusDependenciesV2, WidgetExports} from "../types";
1114

1215
const MockWidgetComponent = ({
1316
text,
@@ -90,7 +93,10 @@ describe("widget-container", () => {
9093
jest.spyOn(console, "error").mockImplementation(() => {});
9194

9295
const onAnalyticsEventSpy = jest.fn();
93-
const depsV2 = {analytics: {onAnalyticsEvent: onAnalyticsEventSpy}};
96+
const depsV2: PerseusDependenciesV2 = {
97+
...testDependenciesV2,
98+
analytics: {onAnalyticsEvent: onAnalyticsEventSpy},
99+
};
94100

95101
registerWidget("mock-widget", MockWidget);
96102

packages/perseus/src/dependencies.ts

+5
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ export type DependencyProps = Partial<
2828

2929
export const DependenciesContext = React.createContext<PerseusDependenciesV2>({
3030
analytics: {onAnalyticsEvent: async () => {}},
31+
useVideo: () => {
32+
throw new Error(
33+
"useVideo dependency not provided in Perseus dependencies",
34+
);
35+
},
3136
});
3237

3338
/**

packages/perseus/src/types.ts

+12-12
Original file line numberDiff line numberDiff line change
@@ -479,18 +479,6 @@ export type PerseusDependencies = {
479479
staticUrl: StaticUrlFn;
480480
InitialRequestUrl: InitialRequestUrlInterface;
481481

482-
// video widget
483-
// This is used as a hook to fetch data about a video which is used to
484-
// add a link to the video transcript. The return value conforms to
485-
// the wonder-blocks-data `Result` type which is used by our GraphQL
486-
// framework.
487-
useVideo(
488-
id: string,
489-
kind: VideoKind,
490-
): Result<{
491-
video: VideoData | null | undefined;
492-
}>;
493-
494482
Log: ILogger;
495483

496484
// RequestInfo
@@ -508,6 +496,18 @@ export type PerseusDependencies = {
508496
*/
509497
export interface PerseusDependenciesV2 {
510498
analytics: {onAnalyticsEvent: AnalyticsEventHandlerFn};
499+
500+
// video widget
501+
// This is used as a hook to fetch data about a video which is used to
502+
// add a link to the video transcript. The return value conforms to
503+
// the wonder-blocks-data `Result` type which is used by our GraphQL
504+
// framework.
505+
useVideo(
506+
id: string,
507+
kind: VideoKind,
508+
): Result<{
509+
video: VideoData | null | undefined;
510+
}>;
511511
}
512512

513513
/**

packages/perseus/src/widgets/video/video-transcript-link.test.tsx

+26-19
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import {render, screen} from "@testing-library/react";
22
import * as React from "react";
33

4-
import {testDependencies} from "../../../../../testing/test-dependencies";
4+
import {testDependenciesV2} from "../../../../../testing/test-dependencies";
55
import * as Dependencies from "../../dependencies";
66

77
import VideoTranscriptLink from "./video-transcript-link";
88

99
describe("VideoTranscriptLink", () => {
1010
beforeEach(() => {
11-
jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
12-
...testDependencies,
11+
jest.spyOn(Dependencies, "useDependencies").mockReturnValue({
12+
...testDependenciesV2,
1313
useVideo: (videoId, kind) => {
1414
if (videoId === "5qap5aO4i9A") {
1515
return {
@@ -135,11 +135,15 @@ describe("VideoTranscriptLink", () => {
135135

136136
it("should link to /transcript/videoNotFound if the URL is not a youtube URL", () => {
137137
// Arrange
138-
jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
139-
...testDependencies,
140-
// @ts-expect-error - TS2322 - Type '(videoId: string, kind: VideoKind) => { status: "success"; data: {}; }' is not assignable to type '(id: string, kind: VideoKind) => Result<{ video: VideoData | null | undefined; }>'.
138+
jest.spyOn(Dependencies, "useDependencies").mockReturnValue({
139+
...testDependenciesV2,
141140
useVideo: (videoId, kind) => {
142-
return {status: "success", data: {}};
141+
return {
142+
status: "success",
143+
data: {
144+
video: null,
145+
},
146+
};
143147
},
144148
});
145149

@@ -152,8 +156,8 @@ describe("VideoTranscriptLink", () => {
152156

153157
it("should handle a success state", () => {
154158
// Arrange
155-
jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
156-
...testDependencies,
159+
jest.spyOn(Dependencies, "useDependencies").mockReturnValue({
160+
...testDependenciesV2,
157161
useVideo: (videoId, kind) => {
158162
return {status: "loading"};
159163
},
@@ -166,11 +170,15 @@ describe("VideoTranscriptLink", () => {
166170

167171
it("should link to /transcript/videoNotFound if there's no data", () => {
168172
// Arrange
169-
jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
170-
...testDependencies,
171-
// @ts-expect-error - TS2322 - Type '(videoId: string, kind: VideoKind) => { status: "success"; data: {}; }' is not assignable to type '(id: string, kind: VideoKind) => Result<{ video: VideoData | null | undefined; }>'.
173+
jest.spyOn(Dependencies, "useDependencies").mockReturnValue({
174+
...testDependenciesV2,
172175
useVideo: (videoId, kind) => {
173-
return {status: "success", data: {}};
176+
return {
177+
status: "success",
178+
data: {
179+
video: null,
180+
},
181+
};
174182
},
175183
});
176184

@@ -183,11 +191,10 @@ describe("VideoTranscriptLink", () => {
183191

184192
it("should handle an error state", () => {
185193
// Arrange
186-
jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
187-
...testDependencies,
188-
// @ts-expect-error - TS2322 - Type '(videoId: string, kind: VideoKind) => { status: "error"; }' is not assignable to type '(id: string, kind: VideoKind) => Result<{ video: VideoData | null | undefined; }>'.
194+
jest.spyOn(Dependencies, "useDependencies").mockReturnValue({
195+
...testDependenciesV2,
189196
useVideo: (videoId, kind) => {
190-
return {status: "error"};
197+
return {status: "error", error: new Error()};
191198
},
192199
});
193200

@@ -198,8 +205,8 @@ describe("VideoTranscriptLink", () => {
198205

199206
it("should handle an aborted state", () => {
200207
// Arrange
201-
jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
202-
...testDependencies,
208+
jest.spyOn(Dependencies, "useDependencies").mockReturnValue({
209+
...testDependenciesV2,
203210
useVideo: (videoId, kind) => {
204211
return {status: "aborted"};
205212
},

packages/perseus/src/widgets/video/video-transcript-link.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {StyleSheet} from "aphrodite";
99
import * as React from "react";
1010

1111
import {usePerseusI18n} from "../../components/i18n-context";
12-
import {getDependencies} from "../../dependencies";
12+
import {useDependencies} from "../../dependencies";
1313

1414
const IS_URL = /^https?:\/\//;
1515

@@ -33,7 +33,7 @@ type Props = {
3333
*/
3434
const VideoTranscriptLink = (props: Props): React.ReactElement => {
3535
const {location} = props;
36-
const {useVideo} = getDependencies();
36+
const {useVideo} = useDependencies();
3737
const [id, kind] = IS_URL.test(location)
3838
? [getYoutubeId(location), "YOUTUBE_ID"]
3939
: [location, "READABLE_ID"];

packages/perseus/src/widgets/video/video.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {testDependencies} from "../../../../../testing/test-dependencies";
1+
import {testDependenciesV2} from "../../../../../testing/test-dependencies";
22
import * as Dependencies from "../../dependencies";
33
import {renderQuestion} from "../__testutils__/renderQuestion";
44

@@ -8,8 +8,8 @@ import type {APIOptions} from "../../types";
88

99
describe("video widget", () => {
1010
beforeEach(() => {
11-
jest.spyOn(Dependencies, "getDependencies").mockReturnValue({
12-
...testDependencies,
11+
jest.spyOn(Dependencies, "useDependencies").mockReturnValue({
12+
...testDependenciesV2,
1313
useVideo: (id, kind) => {
1414
return {
1515
status: "success",

testing/test-dependencies.tsx

+9
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ export const testDependenciesV2: PerseusDependenciesV2 = {
114114
analytics: {
115115
onAnalyticsEvent: async () => {},
116116
},
117+
useVideo: () => {
118+
return {
119+
status: "success",
120+
data: {
121+
video: null,
122+
},
123+
};
124+
},
117125
};
118126

119127
export const storybookTestDependencies: PerseusDependencies = {
@@ -123,6 +131,7 @@ export const storybookTestDependencies: PerseusDependencies = {
123131
};
124132

125133
export const storybookDependenciesV2: PerseusDependenciesV2 = {
134+
...testDependenciesV2,
126135
analytics: {
127136
onAnalyticsEvent: async (event) => {
128137
console.log("⚡️ Sending analytics event:", event);

0 commit comments

Comments
 (0)