From 486e4cdc1196e78101cd9067f37888881fd58f2b Mon Sep 17 00:00:00 2001 From: Matthew Date: Mon, 4 Nov 2024 09:50:46 -0600 Subject: [PATCH] move useVideo to v2 deps (#1793) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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: https://github.com/Khan/perseus/pull/1793 --- .changeset/fuzzy-teachers-scream.md | 6 +++ .../src/multirenderer-editor.tsx | 6 +-- .../src/__tests__/widget-container.test.tsx | 12 +++-- packages/perseus/src/dependencies.ts | 5 +++ packages/perseus/src/types.ts | 24 +++++----- .../video/video-transcript-link.test.tsx | 45 +++++++++++-------- .../widgets/video/video-transcript-link.tsx | 4 +- .../perseus/src/widgets/video/video.test.ts | 6 +-- testing/test-dependencies.tsx | 9 ++++ 9 files changed, 75 insertions(+), 42 deletions(-) create mode 100644 .changeset/fuzzy-teachers-scream.md diff --git a/.changeset/fuzzy-teachers-scream.md b/.changeset/fuzzy-teachers-scream.md new file mode 100644 index 0000000000..8339774fb6 --- /dev/null +++ b/.changeset/fuzzy-teachers-scream.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": major +"@khanacademy/perseus-editor": patch +--- + +Move useVideo from v1 dependency to v2 dependency diff --git a/packages/perseus-editor/src/multirenderer-editor.tsx b/packages/perseus-editor/src/multirenderer-editor.tsx index 76a6902aec..9a9264b1f8 100644 --- a/packages/perseus-editor/src/multirenderer-editor.tsx +++ b/packages/perseus-editor/src/multirenderer-editor.tsx @@ -867,11 +867,11 @@ class MultiRendererEditor extends React.Component { item={item} shape={itemShape} apiOptions={apiOptions} - // Today, with analytics being the only thing in - // dependencies, we send in a dummy function as we don't - // want to gather analytics events from within the editor. + // MultiRenderer is on its way out, + // so I think it's save to use these dummy deps dependencies={{ analytics: {onAnalyticsEvent: async () => {}}, + useVideo: (() => {}) as any, }} > {({renderers}) => ( diff --git a/packages/perseus/src/__tests__/widget-container.test.tsx b/packages/perseus/src/__tests__/widget-container.test.tsx index e37b752a10..a26c43dfe6 100644 --- a/packages/perseus/src/__tests__/widget-container.test.tsx +++ b/packages/perseus/src/__tests__/widget-container.test.tsx @@ -1,13 +1,16 @@ import {render, screen} from "@testing-library/react"; import * as React from "react"; -import {testDependencies} from "../../../../testing/test-dependencies"; +import { + testDependencies, + testDependenciesV2, +} from "../../../../testing/test-dependencies"; import * as Dependencies from "../dependencies"; import WidgetContainer from "../widget-container"; import {registerWidget} from "../widgets"; import PassageWidget from "../widgets/passage"; -import type {WidgetExports} from "../types"; +import type {PerseusDependenciesV2, WidgetExports} from "../types"; const MockWidgetComponent = ({ text, @@ -90,7 +93,10 @@ describe("widget-container", () => { jest.spyOn(console, "error").mockImplementation(() => {}); const onAnalyticsEventSpy = jest.fn(); - const depsV2 = {analytics: {onAnalyticsEvent: onAnalyticsEventSpy}}; + const depsV2: PerseusDependenciesV2 = { + ...testDependenciesV2, + analytics: {onAnalyticsEvent: onAnalyticsEventSpy}, + }; registerWidget("mock-widget", MockWidget); diff --git a/packages/perseus/src/dependencies.ts b/packages/perseus/src/dependencies.ts index 1ec83e83aa..fd7530938a 100644 --- a/packages/perseus/src/dependencies.ts +++ b/packages/perseus/src/dependencies.ts @@ -28,6 +28,11 @@ export type DependencyProps = Partial< export const DependenciesContext = React.createContext({ analytics: {onAnalyticsEvent: async () => {}}, + useVideo: () => { + throw new Error( + "useVideo dependency not provided in Perseus dependencies", + ); + }, }); /** diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index 4630857155..36b50bc14f 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -479,18 +479,6 @@ export type PerseusDependencies = { staticUrl: StaticUrlFn; InitialRequestUrl: InitialRequestUrlInterface; - // video widget - // This is used as a hook to fetch data about a video which is used to - // add a link to the video transcript. The return value conforms to - // the wonder-blocks-data `Result` type which is used by our GraphQL - // framework. - useVideo( - id: string, - kind: VideoKind, - ): Result<{ - video: VideoData | null | undefined; - }>; - Log: ILogger; // RequestInfo @@ -508,6 +496,18 @@ export type PerseusDependencies = { */ export interface PerseusDependenciesV2 { analytics: {onAnalyticsEvent: AnalyticsEventHandlerFn}; + + // video widget + // This is used as a hook to fetch data about a video which is used to + // add a link to the video transcript. The return value conforms to + // the wonder-blocks-data `Result` type which is used by our GraphQL + // framework. + useVideo( + id: string, + kind: VideoKind, + ): Result<{ + video: VideoData | null | undefined; + }>; } /** diff --git a/packages/perseus/src/widgets/video/video-transcript-link.test.tsx b/packages/perseus/src/widgets/video/video-transcript-link.test.tsx index 5efbfa6d03..af84f897c9 100644 --- a/packages/perseus/src/widgets/video/video-transcript-link.test.tsx +++ b/packages/perseus/src/widgets/video/video-transcript-link.test.tsx @@ -1,15 +1,15 @@ import {render, screen} from "@testing-library/react"; import * as React from "react"; -import {testDependencies} from "../../../../../testing/test-dependencies"; +import {testDependenciesV2} from "../../../../../testing/test-dependencies"; import * as Dependencies from "../../dependencies"; import VideoTranscriptLink from "./video-transcript-link"; describe("VideoTranscriptLink", () => { beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue({ - ...testDependencies, + jest.spyOn(Dependencies, "useDependencies").mockReturnValue({ + ...testDependenciesV2, useVideo: (videoId, kind) => { if (videoId === "5qap5aO4i9A") { return { @@ -135,11 +135,15 @@ describe("VideoTranscriptLink", () => { it("should link to /transcript/videoNotFound if the URL is not a youtube URL", () => { // Arrange - jest.spyOn(Dependencies, "getDependencies").mockReturnValue({ - ...testDependencies, - // @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; }>'. + jest.spyOn(Dependencies, "useDependencies").mockReturnValue({ + ...testDependenciesV2, useVideo: (videoId, kind) => { - return {status: "success", data: {}}; + return { + status: "success", + data: { + video: null, + }, + }; }, }); @@ -152,8 +156,8 @@ describe("VideoTranscriptLink", () => { it("should handle a success state", () => { // Arrange - jest.spyOn(Dependencies, "getDependencies").mockReturnValue({ - ...testDependencies, + jest.spyOn(Dependencies, "useDependencies").mockReturnValue({ + ...testDependenciesV2, useVideo: (videoId, kind) => { return {status: "loading"}; }, @@ -166,11 +170,15 @@ describe("VideoTranscriptLink", () => { it("should link to /transcript/videoNotFound if there's no data", () => { // Arrange - jest.spyOn(Dependencies, "getDependencies").mockReturnValue({ - ...testDependencies, - // @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; }>'. + jest.spyOn(Dependencies, "useDependencies").mockReturnValue({ + ...testDependenciesV2, useVideo: (videoId, kind) => { - return {status: "success", data: {}}; + return { + status: "success", + data: { + video: null, + }, + }; }, }); @@ -183,11 +191,10 @@ describe("VideoTranscriptLink", () => { it("should handle an error state", () => { // Arrange - jest.spyOn(Dependencies, "getDependencies").mockReturnValue({ - ...testDependencies, - // @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; }>'. + jest.spyOn(Dependencies, "useDependencies").mockReturnValue({ + ...testDependenciesV2, useVideo: (videoId, kind) => { - return {status: "error"}; + return {status: "error", error: new Error()}; }, }); @@ -198,8 +205,8 @@ describe("VideoTranscriptLink", () => { it("should handle an aborted state", () => { // Arrange - jest.spyOn(Dependencies, "getDependencies").mockReturnValue({ - ...testDependencies, + jest.spyOn(Dependencies, "useDependencies").mockReturnValue({ + ...testDependenciesV2, useVideo: (videoId, kind) => { return {status: "aborted"}; }, diff --git a/packages/perseus/src/widgets/video/video-transcript-link.tsx b/packages/perseus/src/widgets/video/video-transcript-link.tsx index 3eb1216e05..c62a23273d 100644 --- a/packages/perseus/src/widgets/video/video-transcript-link.tsx +++ b/packages/perseus/src/widgets/video/video-transcript-link.tsx @@ -9,7 +9,7 @@ import {StyleSheet} from "aphrodite"; import * as React from "react"; import {usePerseusI18n} from "../../components/i18n-context"; -import {getDependencies} from "../../dependencies"; +import {useDependencies} from "../../dependencies"; const IS_URL = /^https?:\/\//; @@ -33,7 +33,7 @@ type Props = { */ const VideoTranscriptLink = (props: Props): React.ReactElement => { const {location} = props; - const {useVideo} = getDependencies(); + const {useVideo} = useDependencies(); const [id, kind] = IS_URL.test(location) ? [getYoutubeId(location), "YOUTUBE_ID"] : [location, "READABLE_ID"]; diff --git a/packages/perseus/src/widgets/video/video.test.ts b/packages/perseus/src/widgets/video/video.test.ts index 1e07af2b23..347aa1b19a 100644 --- a/packages/perseus/src/widgets/video/video.test.ts +++ b/packages/perseus/src/widgets/video/video.test.ts @@ -1,4 +1,4 @@ -import {testDependencies} from "../../../../../testing/test-dependencies"; +import {testDependenciesV2} from "../../../../../testing/test-dependencies"; import * as Dependencies from "../../dependencies"; import {renderQuestion} from "../__testutils__/renderQuestion"; @@ -8,8 +8,8 @@ import type {APIOptions} from "../../types"; describe("video widget", () => { beforeEach(() => { - jest.spyOn(Dependencies, "getDependencies").mockReturnValue({ - ...testDependencies, + jest.spyOn(Dependencies, "useDependencies").mockReturnValue({ + ...testDependenciesV2, useVideo: (id, kind) => { return { status: "success", diff --git a/testing/test-dependencies.tsx b/testing/test-dependencies.tsx index 96efb8a8eb..004a28587d 100644 --- a/testing/test-dependencies.tsx +++ b/testing/test-dependencies.tsx @@ -114,6 +114,14 @@ export const testDependenciesV2: PerseusDependenciesV2 = { analytics: { onAnalyticsEvent: async () => {}, }, + useVideo: () => { + return { + status: "success", + data: { + video: null, + }, + }; + }, }; export const storybookTestDependencies: PerseusDependencies = { @@ -123,6 +131,7 @@ export const storybookTestDependencies: PerseusDependencies = { }; export const storybookDependenciesV2: PerseusDependenciesV2 = { + ...testDependenciesV2, analytics: { onAnalyticsEvent: async (event) => { console.log("⚡️ Sending analytics event:", event);