convert const enums to regular enums#1430
Open
avacreeth wants to merge 2 commits into
Open
Conversation
summeroff
force-pushed
the
remove_const_enums
branch
from
9af8599 to
a6cfe98
Compare
summeroff
added a commit
that referenced
this pull request
Jun 1, 2026
const enum members are inlined at compile time and produce no runtime object. Downstream consumers that bundle with isolatedModules: true cannot inline across module boundaries, so osn.E*.* reads resolve to undefined at runtime. Dropping const makes tsc emit real runtime enum objects into module.js. Source change folded in from #1430 (Ava Creeth); the js/ outputs were regenerated via the build:javascript pipeline. type_check.js now resolves EPropertyType through the runtime object instead of inlined numeric literals, demonstrating the same inlining fix in this repo's own code. Co-authored-by: Ava Creeth <avacreeth@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
summeroff
added a commit
that referenced
this pull request
Jun 2, 2026
Restores `const enum` for all exported enums. The regular-enum conversion (folded in from #1430) made the enum runtime objects live in module.js, which broke the desktop webpack build: ~11 desktop files import enum values directly from 'obs-studio-node' and relied on const-enum inlining to keep them type-only. As regular enums those imports became runtime value imports, pulling obs-studio-node/module.js (and its native obs_studio_client.node require) into the renderer bundle, which has no node-loader and does not externalize osn. desktop never bundles osn (it loads it at runtime via window.require), so #1430's "bundlers can't inline const enums" rationale does not apply to the primary consumer. Keeping const enums. The js/ build pipeline, generated-file CI check, API-drift fixes, and the typing-accuracy fixes from PR review are unaffected and retained. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make exported enums available at runtime by converting const enum declarations to regular enums, addressing compatibility issues with toolchains that compile TypeScript using isolatedModules: true (where const enum inlining across module boundaries can’t be relied on).
Changes:
- Converted many exported
const enums injs/module.ts/js/module.d.tsto regularenums so they produce runtime objects. - Updated the compiled
js/module.jsto emit and export those runtime enums. - Updated
js/type_check.jsto referenceEPropertyType.*enum members instead of numeric literals.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| js/type_check.js | Switches comparisons from inlined numeric constants to EPropertyType enum members (now requiring ./module at runtime). |
| js/module.ts | Converts a large set of exported const enums to runtime enums. |
| js/module.js | Compiled output updated to include runtime enum objects and exports. |
| js/module.d.ts | Updates enum declarations to match the runtime enums; also includes some additional type/value sync changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+33
to
36
| export enum ESourceFlags { | ||
| Unbuffered = (1 << 0), | ||
| ForceMono = (1 << 1) | ||
| } |
Comment on lines
3
to
+6
| exports.isEmptyProperty = exports.isFontProperty = exports.isCaptureProperty = exports.isColorProperty = exports.isButtonProperty = exports.isBooleanProperty = exports.isEditableListProperty = exports.isListProperty = exports.isPathProperty = exports.isTextProperty = exports.isNumberProperty = void 0; | ||
| const obs = require("./module"); | ||
| function isNumberProperty(property) { | ||
| return property.type === 2 || | ||
| property.type === 3; | ||
| return property.type === obs.EPropertyType.Int || |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This package exports
const enums instead of regularenums. References to aconst enumare inlined at compile time, so there is no runtime object associated with the enum. Most modern javascript bundlers compile typescript withisolatedModules: truewhich means that type checking doesn't happen across module boundaries, and therefore the enums cannot be inlined.Removing the
constkeyword from these enums means that the compiled javascript build object formodule.tswill contain runtime objects that can be used to access enum values.