Skip to content

Commit

Permalink
Refactor ComponentInstance & increase time before timeout warning is …
Browse files Browse the repository at this point in the history
…shown (streamlit#8179)

- Rewrites ComponentInstance from React class component to React functional component
- Warning element when component cannot be loaded is shown after 60 seconds instead of 3 seconds
- A warning log is issued after 15 seconds
  • Loading branch information
raethlein authored and zyxue committed Apr 16, 2024
1 parent 2280aa2 commit 0982fcf
Show file tree
Hide file tree
Showing 8 changed files with 1,091 additions and 447 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,12 @@ describe("Skeleton element", () => {
// writing a very trivial test for it.)
expect(screen.getByTestId("stSkeleton")).toBeVisible()
})

it("renders with height property", () => {
const height = "100px"
render(<Skeleton height={height} />)

const style = getComputedStyle(screen.getByTestId("stSkeleton"))
expect(style.height).toBe(height)
})
})
8 changes: 6 additions & 2 deletions frontend/lib/src/components/elements/Skeleton/Skeleton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ import React, { FC, memo } from "react"

import { SquareSkeleton } from "./styled-components"

const RawSkeleton: FC<React.PropsWithChildren<unknown>> = () => {
return <SquareSkeleton data-testid="stSkeleton" />
interface Props {
height?: string
}

const RawSkeleton: FC<React.PropsWithChildren<Props>> = props => {
return <SquareSkeleton data-testid="stSkeleton" {...props} />
}

export const Skeleton = memo(RawSkeleton)
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,33 @@
* limitations under the License.
*/

import React from "react"
import "@testing-library/jest-dom"
import { act, screen, fireEvent } from "@testing-library/react"

import {
ComponentInstance as ComponentInstanceProto,
SpecialArg,
} from "@streamlit/lib/src/proto"
import "@testing-library/jest-dom"
import { screen, fireEvent } from "@testing-library/react"
import {
DEFAULT_IFRAME_FEATURE_POLICY,
DEFAULT_IFRAME_SANDBOX_POLICY,
} from "@streamlit/lib/src/util/IFrameUtil"
import { logWarning } from "@streamlit/lib/src/util/log"
import { buildHttpUri } from "@streamlit/lib/src/util/UriUtil"
import { WidgetStateManager } from "@streamlit/lib/src/WidgetStateManager"
import React from "react"
import { bgColorToBaseString, toExportedTheme } from "@streamlit/lib/src/theme"
import { fonts } from "@streamlit/lib/src/theme/primitives/typography"
import { mockEndpoints } from "@streamlit/lib/src/mocks/mocks"
import { mockTheme } from "@streamlit/lib/src/mocks/mockTheme"
import {
import { render } from "@streamlit/lib/src/test_util"

import ComponentInstance, {
COMPONENT_READY_WARNING_TIME_MS,
ComponentInstance,
CUSTOM_COMPONENT_API_VERSION,
} from "./ComponentInstance"
import { CUSTOM_COMPONENT_API_VERSION } from "./componentUtils"
import { ComponentRegistry } from "./ComponentRegistry"
import { ComponentMessageType, StreamlitMessageType } from "./enums"
import { render } from "@streamlit/lib/src/test_util"

// Mock log functions.
jest.mock("@streamlit/lib/src/util/log")
Expand Down Expand Up @@ -260,6 +261,115 @@ describe("ComponentInstance", () => {
expect(postMessage).toHaveBeenCalledTimes(2)
})

it("send render message whenever the args change and the component is ready", () => {
let jsonArgs = { foo: "string", bar: 5 }
const componentRegistry = getComponentRegistry()
const { rerender } = render(
<ComponentInstance
element={createElementProp(jsonArgs)}
registry={componentRegistry}
width={100}
disabled={false}
theme={mockTheme.emotion}
widgetMgr={
new WidgetStateManager({
sendRerunBackMsg: jest.fn(),
formsDataChanged: jest.fn(),
})
}
/>
)
const iframe = screen.getByTitle(MOCK_COMPONENT_NAME)
// @ts-expect-error
const postMessage = jest.spyOn(iframe.contentWindow, "postMessage")
// SET COMPONENT_READY
fireEvent(
window,
new MessageEvent("message", {
data: {
isStreamlitMessage: true,
apiVersion: 1,
type: ComponentMessageType.COMPONENT_READY,
},
// @ts-expect-error
source: iframe.contentWindow,
})
)
jsonArgs = { foo: "string", bar: 10 }
rerender(
<ComponentInstance
element={createElementProp(jsonArgs)}
registry={componentRegistry}
width={100}
disabled={false}
theme={mockTheme.emotion}
widgetMgr={
new WidgetStateManager({
sendRerunBackMsg: jest.fn(),
formsDataChanged: jest.fn(),
})
}
/>
)

expect(postMessage).toHaveBeenCalledTimes(2)
})

it("send render message when viewport changes", () => {
const jsonArgs = { foo: "string", bar: 5 }
let width = 100
const componentRegistry = getComponentRegistry()
const { rerender } = render(
<ComponentInstance
element={createElementProp(jsonArgs)}
registry={componentRegistry}
width={width}
disabled={false}
theme={mockTheme.emotion}
widgetMgr={
new WidgetStateManager({
sendRerunBackMsg: jest.fn(),
formsDataChanged: jest.fn(),
})
}
/>
)
const iframe = screen.getByTitle(MOCK_COMPONENT_NAME)
// @ts-expect-error
const postMessage = jest.spyOn(iframe.contentWindow, "postMessage")
// SET COMPONENT_READY
fireEvent(
window,
new MessageEvent("message", {
data: {
isStreamlitMessage: true,
apiVersion: 1,
type: ComponentMessageType.COMPONENT_READY,
},
// @ts-expect-error
source: iframe.contentWindow,
})
)
width = width + 1
rerender(
<ComponentInstance
element={createElementProp(jsonArgs)}
registry={componentRegistry}
width={width}
disabled={false}
theme={mockTheme.emotion}
widgetMgr={
new WidgetStateManager({
sendRerunBackMsg: jest.fn(),
formsDataChanged: jest.fn(),
})
}
/>
)

expect(postMessage).toHaveBeenCalledTimes(2)
})

it("errors on unrecognized API version", () => {
const badAPIVersion = CUSTOM_COMPONENT_API_VERSION + 1
const jsonArgs = { foo: "string", bar: 5 }
Expand Down Expand Up @@ -324,7 +434,7 @@ describe("ComponentInstance", () => {

it("warns if COMPONENT_READY hasn't been received after a timeout", () => {
const componentRegistry = getComponentRegistry()
const { rerender } = render(
render(
<ComponentInstance
element={createElementProp()}
registry={componentRegistry}
Expand All @@ -340,23 +450,8 @@ describe("ComponentInstance", () => {
/>
)
// Advance past our warning timeout, and force a re-render.
jest.advanceTimersByTime(COMPONENT_READY_WARNING_TIME_MS)
act(() => jest.advanceTimersByTime(COMPONENT_READY_WARNING_TIME_MS))

rerender(
<ComponentInstance
element={createElementProp()}
registry={componentRegistry}
width={100}
disabled={false}
theme={mockTheme.emotion}
widgetMgr={
new WidgetStateManager({
sendRerunBackMsg: jest.fn(),
formsDataChanged: jest.fn(),
})
}
/>
)
expect(
screen.getByText(/The app is attempting to load the component from/)
).toBeVisible()
Expand Down Expand Up @@ -539,7 +634,7 @@ describe("ComponentInstance", () => {
const jsonValue = {}
const componentRegistry = getComponentRegistry()
const element = createElementProp(jsonValue)
const { rerender } = render(
render(
<ComponentInstance
element={element}
registry={componentRegistry}
Expand Down Expand Up @@ -583,21 +678,6 @@ describe("ComponentInstance", () => {
})
)

rerender(
<ComponentInstance
element={element}
registry={componentRegistry}
width={100}
disabled={false}
theme={mockTheme.emotion}
widgetMgr={
new WidgetStateManager({
sendRerunBackMsg: jest.fn(),
formsDataChanged: jest.fn(),
})
}
/>
)
// Updating the frameheight intentionally does *not* cause a re-render
// (instead, it directly updates the iframeRef) - so we can't check
// that `child.prop("height") == 100`
Expand Down

0 comments on commit 0982fcf

Please sign in to comment.