Skip to content

Conversation

@JonasBa
Copy link
Member

@JonasBa JonasBa commented Dec 3, 2025

Fixes the type loader signature and uses optional chaining to avoid attempting to access documentation values when the type loader is not enabled.

@JonasBa JonasBa requested review from TkDodo and natemoo-re December 3, 2025 05:36
@JonasBa JonasBa requested review from a team as code owners December 3, 2025 05:36
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Dec 3, 2025
Comment on lines 18 to 28
declare module '!!type-loader!*' {
const TypeLoaderResult: {
props: Record<string, TypeLoader.ComponentDocWithFilename>;
exports: Record<string, {name: string; typeOnly: boolean}[]>;
};
const TypeLoaderResult:
| {
props: Record<string, TypeLoader.ComponentDocWithFilename>;
exports: Record<string, {name: string; typeOnly: boolean}[]>;
}
// If the type loader is not enabled, the return value is an empty module
| undefined;

export default TypeLoaderResult;
}

This comment was marked as outdated.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is wrong

Copy link
Member

Choose a reason for hiding this comment

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

I thought this was legit? The noopTypeLoader would bypass the check here and story.exports.documentation.props would be undefined. The typecasts are obscuring the runtime error.

Copy link
Member Author

Choose a reason for hiding this comment

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

The noop loader returns an empty record, so you get Object.entries({}). The type is wonky because both are records, one just never has values. I used undefined because it is more restrictive as a proxy, so it forces you to handle the missing key, but it's an excessive check

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Looks good! I think the Seer bug might be legit though?

Comment on lines 18 to 28
declare module '!!type-loader!*' {
const TypeLoaderResult: {
props: Record<string, TypeLoader.ComponentDocWithFilename>;
exports: Record<string, {name: string; typeOnly: boolean}[]>;
};
const TypeLoaderResult:
| {
props: Record<string, TypeLoader.ComponentDocWithFilename>;
exports: Record<string, {name: string; typeOnly: boolean}[]>;
}
// If the type loader is not enabled, the return value is an empty module
| undefined;

export default TypeLoaderResult;
}
Copy link
Member

Choose a reason for hiding this comment

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

I thought this was legit? The noopTypeLoader would bypass the check here and story.exports.documentation.props would be undefined. The typecasts are obscuring the runtime error.

Comment on lines 18 to 28
declare module '!!type-loader!*' {
const TypeLoaderResult: {
props: Record<string, TypeLoader.ComponentDocWithFilename>;
exports: Record<string, {name: string; typeOnly: boolean}[]>;
};
const TypeLoaderResult:
| {
props: Record<string, TypeLoader.ComponentDocWithFilename>;
exports: Record<string, {name: string; typeOnly: boolean}[]>;
}
// If the type loader is not enabled, the return value is an empty module
| Record<string, undefined>;

export default TypeLoaderResult;
}

This comment was marked as outdated.

@JonasBa JonasBa merged commit 4eaa6d1 into master Dec 8, 2025
48 checks passed
@JonasBa JonasBa deleted the jb/stories/docs-crash branch December 8, 2025 17:01
ryan953 pushed a commit that referenced this pull request Dec 9, 2025
Fixes the type loader signature and uses optional chaining to avoid
attempting to access documentation values when the type loader is not
enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants