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

tweaks for satisfying typechecker #9

Merged
merged 1 commit into from
Jun 22, 2023
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
2 changes: 1 addition & 1 deletion 3rdparty/lodash/internal/nodeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const freeModule = freeExports && typeof module === 'object' && module !== null
const moduleExports = freeModule && freeModule.exports === freeExports

/** Detect free variable `process` from Node.js. */
const freeProcess = moduleExports && freeGlobal.process
const freeProcess = moduleExports && freeGlobal && freeGlobal.process
Copy link
Contributor Author

Choose a reason for hiding this comment

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

freeGlobal is inferred to have a type of false | typeof globalThis, so we need to ensure that freeGlobal isn't false before checking its properties.


/** Used to access faster Node.js helpers. */
const nodeTypes = ((() => {
Expand Down
2 changes: 1 addition & 1 deletion 3rdparty/preact/signals-core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -660,4 +660,4 @@ function effect(compute: () => unknown): () => void {
return effect._dispose.bind(effect);
}

export { signal, computed, effect, batch, Signal, ReadonlySignal };
export { signal, computed, effect, batch, Signal, type ReadonlySignal };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the identifiers exported on this line are both types and values, but ReadonlySignal is just a type. The type annotation helps bundlers deal with this correctly (read more).

Copy link
Contributor

@CreepGin CreepGin Jun 22, 2023

Choose a reason for hiding this comment

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

Ah I didn't know about this before. A bit more from ChatGPT:

Some third-party JavaScript bundlers like webpack and Rollup might not understand TypeScript's semantics around types, since they don't exist in JavaScript. They might see an import statement and assume it's importing a value that needs to be included in the bundle, even if it's actually only importing a type that gets erased during TypeScript's compilation step.

This can lead to unnecessarily larger bundles, or even errors if a module that only exports types is not included in the bundle because the bundler assumes it needs to be there at runtime.

By using import type or export type, you're making it explicit to both TypeScript and any tooling that understands TypeScript that these imports and exports are type-only, and so they don't need to be included in the JavaScript output. This can help improve bundle size and reduce potential errors.

The TypeScript team actually recommends using import type and export type when you're working in a codebase that also gets built with a bundler, just to be on the safe side and to make your intent clearer to other developers.

46 changes: 23 additions & 23 deletions 3rdparty/preact/signals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,29 @@ const HAS_PENDING_UPDATE = 1 << 0;
const HAS_HOOK_STATE = 1 << 1;
const HAS_COMPUTEDS = 1 << 2;

const enum OptionsTypes {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I goofed on my previous pr (#7) and put these types after the code that uses them, which TypeScript doesn't like.

HOOK = "_hook",
DIFF = "_diff",
DIFFED = "diffed",
RENDER = "_render",
CATCH_ERROR = "_catchError",
UNMOUNT = "unmount",
}

interface OptionsType {
[OptionsTypes.HOOK](component: Component, index: number, type: number): void;
[OptionsTypes.DIFF](vnode: VNode): void;
[OptionsTypes.DIFFED](vnode: VNode): void;
[OptionsTypes.RENDER](vnode: VNode): void;
[OptionsTypes.CATCH_ERROR](error: any, vnode: VNode, oldVNode: VNode): void;
[OptionsTypes.UNMOUNT](vnode: VNode): void;
}

type HookFn<T extends keyof OptionsType> = (
old: OptionsType[T],
...a: Parameters<OptionsType[T]>
) => ReturnType<OptionsType[T]>;

// Install a Preact options hook
function hook<T extends OptionsTypes>(hookName: T, hookFn: HookFn<T>) {
// @ts-ignore-next-line private options hooks usage
Expand Down Expand Up @@ -418,26 +441,3 @@ export function update<T extends SignalOrReactive>(
}
}
*/

const enum OptionsTypes {
HOOK = "_hook",
DIFF = "_diff",
DIFFED = "diffed",
RENDER = "_render",
CATCH_ERROR = "_catchError",
UNMOUNT = "unmount",
}

interface OptionsType {
[OptionsTypes.HOOK](component: Component, index: number, type: number): void;
[OptionsTypes.DIFF](vnode: VNode): void;
[OptionsTypes.DIFFED](vnode: VNode): void;
[OptionsTypes.RENDER](vnode: VNode): void;
[OptionsTypes.CATCH_ERROR](error: any, vnode: VNode, oldVNode: VNode): void;
[OptionsTypes.UNMOUNT](vnode: VNode): void;
}

type HookFn<T extends keyof OptionsType> = (
old: OptionsType[T],
...a: Parameters<OptionsType[T]>
) => ReturnType<OptionsType[T]>;
4 changes: 2 additions & 2 deletions onejs/comps/index.tsx
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { SwitchProps, Switch, ToggleProps, Toggle, DiamondToggle } from "./Toggle"
export { ListboxProps, Listbox, SelectProps, Select } from "./Select"
export { type SwitchProps, Switch, type ToggleProps, Toggle, DiamondToggle } from "./Toggle"
export { type ListboxProps, Listbox, type SelectProps, Select } from "./Select"