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.

Sign up for GitHub

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

Jump to bottom

export client-side APIs from dom-expressions server.js, make them throw an error #345

Merged
merged 4 commits into from
Sep 9, 2024
Merged

export client-side APIs from dom-expressions server.js, make them throw an error #345

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab2d670
export constants from dom-expressions server.js
trusktr Sep 1, 2024
523c89b
export functions from dom-expressions server.js to match client.js ex…
trusktr Sep 1, 2024
bb6fb1c
add missing imports, add missing types
trusktr Sep 1, 2024
fb88f49
Merge branch 'next' into update-exports
ryansolid Sep 9, 2024
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
5 changes: 3 additions & 2 deletions packages/dom-expressions/src/client.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export function dynamicProperty(props: unknown, key: string): unknown;
export function hydrate(
fn: () => JSX.Element,
node: MountableElement,
options?: { renderId?: string, owner?: unknown }
options?: { renderId?: string; owner?: unknown }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier did it

): () => void;
export function getHydrationKey(): string;
export function getNextElement(template?: HTMLTemplateElement): Element;
Expand All @@ -74,4 +74,5 @@ export interface RequestEvent {
request: Request;
}
export declare const RequestContext: unique symbol;
export function getRequestEvent(): RequestEvent | undefined;
export function getRequestEvent(): RequestEvent | undefined;
export function runHydrationEvents(): void;
2 changes: 1 addition & 1 deletion packages/dom-expressions/src/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ function insertExpression(parent, value, current, marker, unwrapArray) {
if (marker === undefined) return (current = [...parent.childNodes]);
let node = array[0];
if (node.parentNode !== parent) return current;
const nodes = [node]
const nodes = [node];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier did it

while ((node = node.nextSibling) !== marker) nodes.push(node);
return (current = nodes);
}
Expand Down
88 changes: 88 additions & 0 deletions packages/dom-expressions/src/server.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
import { JSX } from "./jsx.js";
export const Aliases: Record<string, string>;
export const Properties: Set<string>;
export const ChildProperties: Set<string>;
export const DelegatedEvents: Set<string>;
export const DOMElements: Set<string>;
export const SVGElements: Set<string>;
export const SVGNamespace: Record<string, string>;
export function getPropAlias(prop: string, tagName: string): string | undefined;

type MountableElement = Element | Document | ShadowRoot | DocumentFragment | Node;

export function renderToString<T>(
fn: () => T,
options?: {
Expand Down Expand Up @@ -87,3 +99,79 @@ export function pipeToNodeWritable<T>(
onCompleteAll?: () => void;
}
): void;

export function untrack<T>(fn: () => T): T;

// client-only APIs

/** @deprecated not supported on the server side */
export function classList(
node: Element,
value: { [k: string]: boolean },
prev?: { [k: string]: boolean }
): { [k: string]: boolean };

/** @deprecated not supported on the server side */
export function style(
node: Element,
value: { [k: string]: string },
prev?: { [k: string]: string }
): void;

/** @deprecated not supported on the server side */
export function insert<T>(
parent: MountableElement,
accessor: (() => T) | T,
marker?: Node | null,
init?: JSX.Element
): JSX.Element;

/** @deprecated not supported on the server side */
export function spread<T>(
node: Element,
accessor: (() => T) | T,
isSVG?: Boolean,
skipChildren?: Boolean
): void;

/** @deprecated not supported on the server side */
export function delegateEvents(eventNames: string[], d?: Document): void;
/** @deprecated not supported on the server side */
export function dynamicProperty(props: unknown, key: string): unknown;
/** @deprecated not supported on the server side */
export function setAttribute(node: Element, name: string, value: string): void;
/** @deprecated not supported on the server side */
export function setAttributeNS(node: Element, namespace: string, name: string, value: string): void;

/** @deprecated not supported on the server side */
export function addEventListener(
node: Element,
name: string,
handler: () => void,
delegate: boolean
): void;

/** @deprecated not supported on the server side */
export function render(code: () => JSX.Element, element: MountableElement): () => void;
/** @deprecated not supported on the server side */
export function template(html: string, isCE?: boolean, isSVG?: boolean): () => Element;
/** @deprecated not supported on the server side */
export function setProperty(node: Element, name: string, value: any): void;
/** @deprecated not supported on the server side */
export function className(node: Element, value: string): void;
/** @deprecated not supported on the server side */
export function assign(node: Element, props: any, isSVG?: Boolean, skipChildren?: Boolean): void;

/** @deprecated not supported on the server side */
export function hydrate(
fn: () => JSX.Element,
node: MountableElement,
options?: { renderId?: string; owner?: unknown }
): () => void;

/** @deprecated not supported on the server side */
export function getNextElement(template?: HTMLTemplateElement): Element;
/** @deprecated not supported on the server side */
export function getNextMatch(start: Node, elementName: string): Element;
/** @deprecated not supported on the server side */
export function getNextMarker(start: Node): [Node, Array<Node>];
46 changes: 44 additions & 2 deletions packages/dom-expressions/src/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
import { Aliases, BooleanAttributes, ChildProperties } from "./constants";
import { Aliases, BooleanAttributes, ChildProperties, Properties } from "./constants";
import { sharedConfig, root } from "rxcore";
Copy link
Contributor Author

@trusktr trusktr Sep 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import was missing, see further below in the file where Properties.has(prop) was being called, but Properties was not defined (TypeScript is not type checking these JS files).

import { createSerializer, getLocalHeaderScript } from "./serializer";
export { createComponent } from "rxcore";

export { getOwner, createComponent, effect, memo, untrack } from "rxcore";

Copy link
Contributor Author

@trusktr trusktr Sep 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this list accurate? I based this off of web/src/core.ts, which I assume indicates which APIs are available server-side, but I did not include those that were already included here in server.js (f.e. mergeProps is defined here in server.js).

Goal is to bring server.js exports to closer parity with client.js exports.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, no, not accurate. The code is difficult to follow.

I see that the build output has code injected, that I need to avoid clashing with:

const isServer = true;
const isDev = false;
function render() {}
function hydrate() {}
function insert() {}
function spread() {}
function addEventListener() {}
function delegateEvents() {}
function Dynamic(props) {
  const [p, others] = splitProps(props, ["component"]);
  const comp = p.component,
    t = typeof comp;
  if (comp) {
    if (t === "function") return comp(others);else if (t === "string") {
      return ssrElement(comp, others, undefined, true);
    }
  }
}
function Portal(props) {
  return "";
}

Copy link
Contributor Author

@trusktr trusktr Sep 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this list of exports? For example function render() {} is injected, and that is not exported in server.d.ts.

Scratch that. I was confused because of dom-expressions and rxcore indirection. Hard to follow, IDE intellisense has no idea what is happening (therefore me too lol).

Copy link
Contributor Author

@trusktr trusktr Sep 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is confusing because it looks like the exports from dom-expressions get merged with the exports from web/server/index.ts into a single file.

I know what to do now...

export {
Properties,
ChildProperties,
getPropAlias,
Aliases,
DOMElements,
SVGElements,
SVGNamespace,
DelegatedEvents
} from "./constants.js";

// Based on https://github.com/WebReflection/domtagger/blob/master/esm/sanitizer.js
const VOID_ELEMENTS =
Expand Down Expand Up @@ -674,3 +686,33 @@ export function ssrSpread(props, isSVG, skipChildren) {
}
return result;
}

// client-only APIs

export {
notSup as classList,
notSup as style,
notSup as insert,
notSup as spread,
notSup as delegateEvents,
notSup as dynamicProperty,
notSup as setAttribute,
notSup as setAttributeNS,
notSup as addEventListener,
notSup as render,
notSup as template,
notSup as setProperty,
notSup as className,
notSup as assign,
notSup as hydrate,
notSup as getNextElement,
notSup as getNextMatch,
notSup as getNextMarker,
notSup as runHydrationEvents
};

Copy link
Contributor Author

@trusktr trusktr Sep 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And are these accurate? Wondering if I missed anything. Maybe this is good enough to start with at least. The Aliases errors are gone when importing solid-js/html server-side for example, which is good enough for my case.

Copy link
Contributor Author

@trusktr trusktr Sep 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are all new functions never exported before for server, so it is non-breaking to make them errors I believe. However the question is, do you want any of them to be no-ops instead of error-throwing? I think errors are better for DX to inform people what not to do.

Also, any of these functions can be overriden to be no-ops over in solid-js, for example like some already are here:

https://github.com/solidjs/solid/pull/2269/files#r1740197787

function notSup() {
throw new Error(
"Client-only API called on the server side. Run client-only code in onMount, or conditionally run client-only component with <Show>."
);
}
Loading