Skip to content

fix(widgets): Remove src import#9833

Merged
ibgreen merged 1 commit intomasterfrom
ib/fix-widget-import
Oct 17, 2025
Merged

fix(widgets): Remove src import#9833
ibgreen merged 1 commit intomasterfrom
ib/fix-widget-import

Conversation

@ibgreen
Copy link
Collaborator

@ibgreen ibgreen commented Oct 17, 2025

Closes #9832

Background

Change List

@coveralls
Copy link

Coverage Status

coverage: 91.144%. remained the same
when pulling 10fd0cb on ib/fix-widget-import
into 97c651f on master.

@ibgreen ibgreen added this to the v9.2 patch releases milestone Oct 17, 2025
@ibgreen ibgreen added the bug label Oct 17, 2025
import {IconButton} from './lib/components/icon-button';

/** @note Mirrors an internal calss in deck.gl/core. We can easily redefine it here */
type ViewOrViews = View | View[] | null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It’s probably not ideal for a new user to have to dig into the source to find this type definition.

I’d suggest exporting ViewOrViews from core, since it’s used in the public Widget class and the intent is to mirror how Deck props are typed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

haha, yes, I anticipated this comment.

As usual, I'm stuck in my old school view, where a problem with typescript is that through this type of issues, suddenly users want little helper types exported, leading to big, often unaudited increases in the supported public API.

This one really sounded like an internal helper so I opted not to export it.

I guess I could even have avoided redefining it, and just typed it out where needed.

Oh well...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I figured this was probably intentional on your part. And given that ViewOrViews is an older, stable type, it’s unlikely to change.

To your point, widgets really stress the API.. similar to writing layers or effects from scratch. Suddenly there’s a pull to externalize internal APIs because widget authors are effectively changing Deck’s top-level behavior.

One key difference I’m seeing vs. our other components is that widgets often override Deck behavior (e.g., taking control of initialViewState). That’s the opportunity, but also why we’ve struggled.

Would it be worth documenting where we draw the line here - maybe a page in the unfinished “custom widget” guide that outlines what’s supported, what’s discouraged, and where the boundaries are?

@ibgreen ibgreen merged commit 5b2325d into master Oct 17, 2025
5 checks passed
@ibgreen ibgreen deleted the ib/fix-widget-import branch October 17, 2025 19:34
ibgreen added a commit that referenced this pull request Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] widget module causes tons of typescript errors

3 participants