Skip to content

Commit

Permalink
move useVideo to v2 deps (#1793)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
handeyeco authored Nov 4, 2024
1 parent 22d1c02 commit 486e4cd
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 42 deletions.
6 changes: 6 additions & 0 deletions .changeset/fuzzy-teachers-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": major
"@khanacademy/perseus-editor": patch
---

Move useVideo from v1 dependency to v2 dependency
6 changes: 3 additions & 3 deletions packages/perseus-editor/src/multirenderer-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -867,11 +867,11 @@ class MultiRendererEditor extends React.Component<MultiRendererEditorProps> {
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}) => (
Expand Down
12 changes: 9 additions & 3 deletions packages/perseus/src/__tests__/widget-container.test.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);

Expand Down
5 changes: 5 additions & 0 deletions packages/perseus/src/dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ export type DependencyProps = Partial<

export const DependenciesContext = React.createContext<PerseusDependenciesV2>({
analytics: {onAnalyticsEvent: async () => {}},
useVideo: () => {
throw new Error(
"useVideo dependency not provided in Perseus dependencies",
);
},
});

/**
Expand Down
24 changes: 12 additions & 12 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}>;
}

/**
Expand Down
45 changes: 26 additions & 19 deletions packages/perseus/src/widgets/video/video-transcript-link.test.tsx
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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,
},
};
},
});

Expand All @@ -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"};
},
Expand All @@ -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,
},
};
},
});

Expand All @@ -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()};
},
});

Expand All @@ -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"};
},
Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/widgets/video/video-transcript-link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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?:\/\//;

Expand All @@ -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"];
Expand Down
6 changes: 3 additions & 3 deletions packages/perseus/src/widgets/video/video.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -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",
Expand Down
9 changes: 9 additions & 0 deletions testing/test-dependencies.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,14 @@ export const testDependenciesV2: PerseusDependenciesV2 = {
analytics: {
onAnalyticsEvent: async () => {},
},
useVideo: () => {
return {
status: "success",
data: {
video: null,
},
};
},
};

export const storybookTestDependencies: PerseusDependencies = {
Expand All @@ -123,6 +131,7 @@ export const storybookTestDependencies: PerseusDependencies = {
};

export const storybookDependenciesV2: PerseusDependenciesV2 = {
...testDependenciesV2,
analytics: {
onAnalyticsEvent: async (event) => {
console.log("⚡️ Sending analytics event:", event);
Expand Down

0 comments on commit 486e4cd

Please sign in to comment.