Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix/custom component basepath #176

Merged
merged 2 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/playground/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function App() {
entrypoint: "Hello.py",
requirements: DEFAULT_REQUIREMENTS,
files: DEFAULT_KERNEL_FILES,
basePath: __webpack_public_path__,
});
setKernel(kernel);

Expand Down
1 change: 1 addition & 0 deletions packages/playground/src/declarations.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare let __webpack_public_path__: string | undefined; // Ref: https://webpack.js.org/guides/public-path/#on-the-fly
23 changes: 23 additions & 0 deletions packages/stlite-kernel/src/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ import Worker from "!!worker-loader?inline=no-fallback!./worker";

let httpCommId = 0;

// Ref: https://github.com/streamlit/streamlit/blob/1.12.2/frontend/src/lib/UriUtil.ts#L32-L33
const FINAL_SLASH_RE = /\/+$/;
const INITIAL_SLASH_RE = /^\/+/;

export interface StliteKernelOptions {
/**
* The Streamlit subcommand to run.
Expand Down Expand Up @@ -45,6 +49,19 @@ export interface StliteKernelOptions {
*
*/
wheelBaseUrl?: string;

/**
* The `pathname` that will be used as both
* a base path of the custom component URLs
* ana the path of the main page in MPA.
*
* If not specified, the value of `window.location.pathname` at the time of the class initialization is used.
* This default is good enough for most cases, however,
* it may be a problem if the server is configured to return the main page even if the URL is pointing to the subpath.
* In such a setting, problems like https://github.com/whitphx/stlite/issues/171 may happen,
* so explicitly setting `basePath` is recommended.
*/
basePath?: string;
}

export class StliteKernel {
Expand All @@ -56,7 +73,13 @@ export class StliteKernel {

private _workerInitData: WorkerInitialData;

public readonly basePath: string; // TODO: Move this prop to outside this class. This is not a member of the kernel business logic, but just a globally referred value.

constructor(options: StliteKernelOptions) {
this.basePath = (options.basePath ?? window.location.pathname)
.replace(FINAL_SLASH_RE, "")
.replace(INITIAL_SLASH_RE, "");

this._worker = new Worker();
this._worker.onmessage = (e) => {
this._processWorkerMessage(e.data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ const CustomComponentIFrame = React.forwardRef<
HTMLIFrameElement,
CustomComponentIFrameProps
>((props, ref) => {
const path = extractCustomComponentPath(window.location.pathname, props.src);
const kernel = useStliteKernel();
const path = extractCustomComponentPath(kernel.basePath, props.src);

if (path == null) {
return <iframe {...props} ref={ref} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,25 @@ describe("extractCustomComponentPath", () => {
expected: string;
}[] = [
{
basePathname: "/",
basePathname: "",
url: "http://xxx:99999/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2F",
expected:
"/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2F",
},
{
basePathname: "/stlite",
basePathname: "stlite",
url: "http://xxx:99999/stlite/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2Fstlite",
expected:
"/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2Fstlite",
},
{
basePathname: "/stlite/index.html",
basePathname: "stlite/index.html",
url: "http://xxx:99999/stlite/index.html/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2Fstlite%2Findex.html",
expected:
"/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2Fstlite%2Findex.html",
},
{
basePathname: "/index.html",
basePathname: "index.html",
url: "http://xxx:99999/index.html/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2Findex.html",
expected:
"/component/package.component/index.html?streamlitUrl=http%3A%2F%2Flocalhost%3A3000%2Findex.html",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { DUMMY_BASE_HOST } from "../../consts";

export function extractCustomComponentPath(
basePathname: string,
basePath: string, // basePath without leading and trailing slashes.
url: string
): string | null {
const baseHostAndPath = (DUMMY_BASE_HOST + basePathname).replace(/\/$/, "");
const baseHostAndPath = (DUMMY_BASE_HOST + "/" + basePath).replace(/\/$/, "");
const regex = new RegExp(`https?://${baseHostAndPath}(/.*?$)`);

const matches = url.match(regex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,50 +60,24 @@ export class ConnectionManager {
return this.connectionState === ConnectionState.CONNECTED
}

/**
* In MPA, `pathname` changes over the pages,
* so here the first obtained `pathname` is cached to be used
* as a consistent `basePath` over the all page accesses,
* assuming that the first time when this method is called is
* when the main page is accessed whose `pathname` is the `basePath`.
*
* This assumption is supported because the time when this frontend script is loaded and executed
* is when the user accessed the main page, and it is when this method is called for the first time.
*
* Sometimes unfortunately this assumption is broken, for example,
* when the server is configured to return the main page even if the URL is pointing to the subpath.
* It can happen because, on stlite, the MPA system works only on the frontend
* and the static HTTP server does not know about the page structure of the MPA.
*/
private basePathCache: string | null = null
private getBasePath(): string {
if (this.basePathCache == null) {
this.basePathCache = (window.location.pathname || "").replace(/^\//, "")
}
return this.basePathCache
}

/**
* Return the BaseUriParts for the server we're connected to,
* if we are connected to a server.
*/
public getBaseUriParts(): BaseUriParts | undefined {
if (this.connectionState === ConnectionState.CONNECTED) {
// The existence of this property became necessary for multi-page apps: https://github.com/streamlit/streamlit/pull/4698/files#diff-e56cb91573ddb6a97ecd071925fe26504bb5a65f921dc64c63e534162950e1ebR967-R975
// so here a dummy BaseUriParts object is returned though it may not be compatible with multi-page functionality.
// For multi-page app compatibility,
// See https://github.com/streamlit/streamlit/blob/ace58bfa3582d4f8e7f281b4dbd266ddd8a32b54/frontend/src/lib/UriUtil.ts#L60-L72
// or its caller https://github.com/streamlit/streamlit/blob/ace58bfa3582d4f8e7f281b4dbd266ddd8a32b54/frontend/src/lib/ConnectionManager.ts#L142
// so here a dummy BaseUriParts object is returned.
// The host and port are set as dummy values that are invalid as a URL
// in order to avoid unexpected accesses to external resources,
// while the basePath is representing the actual info.
return {
host: DUMMY_BASE_HOSTNAME,
port: DUMMY_BASE_PORT,
// When a new session starts, a page name for multi-page apps (a relative path to the app root url) is calculated based on this `basePath`
// then a `rerunScript` BackMsg is sent to the server with `pageName` (https://github.com/streamlit/streamlit/blob/ace58bfa3582d4f8e7f281b4dbd266ddd8a32b54/frontend/src/App.tsx#L1064)
// and `window.history.pushState` is called (https://github.com/streamlit/streamlit/blob/ace58bfa3582d4f8e7f281b4dbd266ddd8a32b54/frontend/src/App.tsx#L665),
// so here `window.location.pathname` must be set here as `basePath` representing the app root url path.
// With this implementation, multi-page apps are not supported on stlite.
// See https://github.com/whitphx/stlite/issues/41
basePath: this.getBasePath(),
// and `window.history.pushState` is called (https://github.com/streamlit/streamlit/blob/ace58bfa3582d4f8e7f281b4dbd266ddd8a32b54/frontend/src/App.tsx#L665).
basePath: this.props.kernel.basePath,
}
}
return undefined
Expand Down