Skip to content

Create a Globalized Type - #2342

Closed
Adam Naji (Bashamega) wants to merge 2 commits into
microsoft:mainfrom
Bashamega:globalized
Closed

Create a Globalized Type#2342
Adam Naji (Bashamega) wants to merge 2 commits into
microsoft:mainfrom
Bashamega:globalized

Conversation

@Bashamega

Copy link
Copy Markdown
Contributor

This PR introduces a reusable generic utility type, Globalized, to model environments where a scoped global (such as WorkerGlobalScope) is augmented with globalThis.
This will allow us to use type instead of override type when using it in KDL or JSONc

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the PR!

This section of the codebase is owned by Kagami Sascha Rosylight (@saschanaz) - if they write a comment saying "LGTM" then it will be merged.

@jakebailey

Copy link
Copy Markdown
Member

I don't think we should just be adding random global types like this that aren't from a spec; this seems like it's just the same code but not inlined?

@Bashamega

Adam Naji (Bashamega) commented Dec 31, 2025

Copy link
Copy Markdown
Contributor Author

We already created a similar type in #2338 for SelectionDirection

@jakebailey

Copy link
Copy Markdown
Member

That's totally different,no?

@Bashamega

Copy link
Copy Markdown
Contributor Author

That's totally different,no?

I think no, I think this is just using the DRY method like that pr

@saschanaz

Copy link
Copy Markdown
Contributor

Adding string enum is OK-ish as users may want to use the same definition without repeating, but in this case it's unlikely that users would ever do this manually.

@Bashamega

Copy link
Copy Markdown
Contributor Author

Understood Kagami Sascha Rosylight (@saschanaz)

@saschanaz

Copy link
Copy Markdown
Contributor

(also the spec would use string enum if it was added in 2025; it does not because it's added back in the old days when it wasn't using Web IDL and doing so now would cause potential webcompat issue because of order of exception)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants