ReviewOS

home-lang/home

fix(ts): infer CommonJS function constructors and nested this types

#966
Open glennmichael123 opened this 5 days ago ·
5 days ago

Verified checkpoint (2026-10-29)

TypeScript 5.7.3 and Home agree exactly for the CommonJS constructor CLI fixtures, including chained prototype assignments, in both checkJs modes. The wider admitted-JavaScript oracle audit is 134/134 exact stdout, stderr, and exit matches. Commits c150e32bb, 3064adfa3, and 0cfda77e1 are on origin/main.

The latest guarded home-tsc build passed with a 2,158 MB peak counted tree footprint. The focused chained CommonJS Program test passed with a 2,094 MB peak. The new fixture emits the same two TS2345 argument errors as TypeScript in both modes. See the latest verified progress comment.

Remaining acceptance

Run the broader checker and Program suites, adjudicate any legacy constructor expectations against the pinned TypeScript 5.7.3 control, and verify source-revision/editor invalidation behavior where this constructor shape can change. Keep #846 and #66 linked. This issue remains open until those checks are complete; the locally missing pinned WebKit artifact also limits runtime parity claims.

6 hours ago

2026-11-02: 0632dcbd6 is pushed to origin/main as Glenn without coauthors. The new LSP regression changes a chained CommonJS prototype method's JSDoc parameter from number to string, removes the method, then restores it at the same owner path. It asserts both imported constructor instances republish TS2345 -> no diagnostic -> TS2339 -> TS2345 in both checkJs modes, with no duplicate publication.

The focused zig build test-ts-lsp '-Dts-lsp-test-filter=chained CommonJS prototype edits republish constructor importer diagnostics' -j1 passed under the authorized 3,840 MB guard (1,957 MB peak counted tree footprint; 47% host low-water). The umbrella -Dfilter=ts_lsp had also selected ts_lsp_server and caused two compiler processes to exceed the guard; using the dedicated package step runs the intended LSP test. Earlier focused Program tests and 134/134 exact TypeScript 5.7.3 admitted-JS CLI streams also passed. Broader checked-JS constructor gaps remain tracked in #968.

6 hours ago

Pushed 316fb86ce to origin/main as glennmichael123 without coauthors. Its CommonJS whole-export sub-function checker control now asserts the exact TypeScript 5.7.3 result: zero diagnostics, including no contextual-this, missing-instance-property, or implicit-construction error. The Home control passed in the full guarded checker run. The previously pushed chained CommonJS constructor LSP edit/revert regression remains verified. The broader checker run, including local #968 work, was 4,708 pass / 8 fail / 4,716 total; those remaining gaps are tracked in #968. The 134-stream CLI audit for this batch remains pending.

2 hours ago

2026-11-02 checkpoint: f39287d68 is on origin/main and extends nested CommonJS export shapes from actual assignment members. It also distinguishes an inferred object literal export from an explicitly typed or scalar export when checking nested writes. Independent TypeScript 5.7.3 controls establish zero diagnostics for the inferred two-file case and three TS2339 diagnostics for the declared/scalar negatives. Home's focused tests and the related CommonJS checker group pass 52/52. 801061ed5 also fixes a linked constructor prototype JSDoc field case (#968).

Source-revision LSP coverage, a fresh full checker run, and the 134-stream CLI audit remain to be done before closure. Related: #968, #846, #66.

2 hours ago

2026-11-02 LSP revision test checkpoint, linked to #968 and #846:

A new two-file /owner.js + /consumer.ts test exercises exports.n = {}; exports.n.K = function () { this.x = 10; }; through import mod = require('./owner.js'), then removes, annotates, and restores the nested constructor. The exact TypeScript 5.7.3 control has no initial diagnostic in either checkJs mode. Home currently reports TS2339 (Property 'K' does not exist on type '{}') at the initial import with checkJs=false.

The failure is in Program's owner-schema projection: checked_schema.namedExportTypes records the direct exports.n = {} value but does not fold the later exports.n.K assignment into that named export's imported type. The in-file/virtual CommonJS checker path already handles the nested write; the Program import path needs the same source-order semantics, including closed JSDoc receiver types and editor revision invalidation. The focused LSP test is left failing until this is corrected.

The latest Zig LSP test was guarded at 3,840 MB, peaked at 2,124 MB counted tree footprint, and the host low-water mark was 41%. Host memory has since fallen below the required 40%, so additional Zig runs are paused.

2 hours ago

Pushed bd3371310 to origin/main as Glenn, with no coauthors. It ports nested CommonJS export-member collection through the checker, Program schema, and LSP publication path. The schema now extends inferred exports.n = {} objects with later exports.n.K writes in source order, unions repeated new-member writes, and keeps JSDoc-declared or object-literal-declared receivers constrained.

Verified on the exact staged snapshot under the 3,840 MB guard: the focused Program source-order test, the LSP four-revision importer diagnostics test in both checkJs modes, and the checker CommonJS regression group all pass. Peak counted process-tree footprint was 2,123 MB or less in these runs. HEAD and origin/main both resolve to this commit. The broader checker suite and 134-stream CLI audit are still pending under #846; checked-JS constructor follow-up remains under #968.

2 hours ago

Constructor parity checkpoint: 90b9d6325 is on main and fixes same-line JSDoc @template/@param scanning. The standalone parser suite passed 23/23 and the exact Box<number> member-substitution regression passed under the 3,840 MB guard. The full checker run after that commit was 4,716 pass / 10 fail (4,726 total), so this constructor issue remains open.

An exact TypeScript 5.7.3 control for C<T> writing this.x, replacing C.prototype with { method() { return this.x; } }, and declaring @type {C<number>} exits with zero diagnostics. Home still has a template-specific gate in its prototype object member diagnostic reconciliation that treats a real constructor field as missing. I removed that gate locally so generic constructors use the same resolved instance members as nongeneric constructors; the focused Home test has not run yet because the guard refused launches at 38–39% host memory (required: >=40). No checker commit or push is claimed for this local change.

4 days ago

The focused reproducer is committed on origin/main as 374f0c2d3. #965 removes two unrelated expando TS2339s, but this constructor contextual-this / construct-signature differential remains open and is excluded from the passing #965 oracle audit.

4 days ago

Further isolated with real files and allowJs/checkJs/strict/noEmit under TypeScript 5.7.3: (1) a lone module.exports = function(p) { this.t = ... } imported via new Main is clean in TS, while Home emits owner TS2683 and importer TS7009; (2) a lone module.exports.Sub = function() { this.t = 1 } imported via new Main.Sub is clean in TS, while Home emits owner TS2339 and importer TS7009; (3) the nested form is also clean in TS but combines both failures in Home. This confirms #966 spans instance-this inference and construct-signature projection for both whole and named CommonJS function assignments. TypeScript 5.7.3's local isJSConstructor recognizes a function expression whose symbol has instance members; Home's current thisInsideCheckJsConstructorFunction excludes named CommonJS assignment functions. No patch or suppression was applied here.

4 days ago

Three isolated exact TypeScript 5.7.3 controls are now on origin/main: whole-function constructor 35194c876, named module.exports.Sub constructor c83db1199, and JSDoc @constructor named export 4868b8db3. All three exit 0 with empty stdout/stderr in TS5.7.3. Current Home exits 2: whole emits importer TS7009 + owner TS2683; named emits importer TS7009 + owner TS2339; tagged emits importer TS7009 + owner TS2339. The existing combined fixture remains red. This separates instance-this inference from construct-signature projection, and shows that the current named-export receiver type is wrong even with @constructor. No failing controls were added to the passing #846/#965 auditor. Local TS5.7.3 source's isJSConstructor uses the function symbol's instance members; Home currently has fnLooksLikeCheckJsConstructor and assignedFunctionExpressionEffectiveThis paths that do not model these CommonJS assigned functions. The exact implementation still needs owner and importer type transport; #966 stays open.

4 days ago

Fresh exact TypeScript 5.7.3 differential on the four existing real-file configs in packages/ts_cli/testdata/issue966-commonjs-constructor, each run with checkJs=true and false under the approved guard (peak 203 MB; host low-water 41%): tsc exits 0 with empty streams in all 8 cases; Home exits 2 in all 8.

  • tsconfig.json (whole constructor plus nested .Sub): Home emits importer TS7009 in both modes; checked owner also emits TS2683, TS2339 on this.instance, and TS7009 on new module.exports.
  • tsconfig-whole.json: Home emits importer TS7009 in both modes; checked owner also emits TS2683.
  • tsconfig-named.json: Home emits importer TS7009 in both modes; checked owner also emits TS2339 on this.t (receiver rendered as typeof import("named-owner")).
  • tsconfig-tagged.json: Home emits importer TS7009 in both modes; checked owner also emits TS2339 on this.x (receiver rendered as typeof import("tagged-owner")).

TypeScript 5.7.3's local compiler source classifies a JS function as a constructor when its symbol has inferred instance members (or a JSDoc class tag), then turns its call signature into a construct signature returning the inferred class instance. Home currently interns a plain call signature in checkFnSignatureOnly, and checked_schema.zig projects that signature as a callable function. This points to a checker and Program schema change, including contextual this, rather than dropping TS7009/TS2683/TS2339. No #966 patch is claimed yet. Related #846 and #66; #965 and #967 are now closed.

4 days ago

Progress on admitted JavaScript CommonJS constructors (#846): the TypeScript 5.7.3 CLI oracle is clean in all eight checkJs/fixture combinations. Home currently passes the two whole-export combinations; the six named/nested/tagged combinations still need parity.

Local uncommitted work now infers constructor instances and construct signatures for inline module.exports = function and module.exports.Sub = function expressions. A targeted trace found that the remaining owner-side TS2339 was emitted by reportUnsupportedNestedCommonJsFunctionThisMembers: it compared this fields against the module export object's static shape, even after the constructor instance had been inferred. I have corrected that path and added focused checked-schema and Program tests for named/nested imports. The named import's TS7009 and prototype replacement semantics remain under investigation.

The newest patch has not been pushed: the kernel memory admission level is around 30%, below the 40% threshold used for sequential guarded Zig builds. Earlier guarded builds passed with peak counted tree footprint about 2.3 GB; the latest owner-path and prototype changes still require a build and differential run. I will commit and push only after those checks pass.

4 days ago

Verified progress in c150e32bb (pushed to origin/main): inline JavaScript CommonJS function constructors now publish their instance members and construct signatures to admitted importers. The checker also distinguishes the constructor's this from the module export object, permits prototype-object replacement on a constructible named export, and lets an actual checked construct signature override the resolver's earlier call-only syntax fact.

Evidence: guarded zig build home-tsc -j1 passed (peak counted tree footprint 2,152 MB); the focused checked-schema and Program constructor tests passed; the four #966 CLI fixtures with checkJs both on and off matched TypeScript 5.7.3 exactly, 8/8; the expanded #846 oracle audit matched 132/132 configurations with exact stdout, stderr, and exit codes. All runs used the 3,840 MB guard and 40% host admission level.

I am keeping this issue open until the broader checker/Program suites and revision behavior are checked. The issue66 native WebKit pin is still unavailable locally, so the Home build warns that it links system JavaScriptCore; that limits runtime conclusions, but these compiler oracle checks passed.

4 days ago

Pushed two #966 commits directly to origin/main: 3064adfa3 projects methods from chained CommonJS prototype assignments into both exported constructor instance types, and 0cfda77e1 adds a focused Program regression in both checkJs modes. The prototype collector now obtains function signatures before a later assignment is visited, without checking method bodies twice.

Verified against TypeScript 5.7.3: the new chained fixture emits exactly two TS2345 argument errors in both modes, and the admitted-JavaScript CLI auditor reports 134/134 exact stdout, stderr, and exit matches. Guarded zig build home-tsc -j1 passed (2,158 MB peak counted tree footprint); the focused Program test passed (2,094 MB peak). Both commits use glennmichael123 <gtorregosa@gmail.com> with no coauthor trailers.

Broader checker/Program suites and source-revision behavior remain open acceptance items. Host memory returned below the 40% Zig start threshold after the focused run, so those tests are queued rather than claimed as passed. This remains a verified subset of #846.

4 days ago

Verified #966 progress on origin/main: e0e5e0983 preserves inferred construct signatures when a virtual multi-file CommonJS owner assigns a function to exports.Ctor. It moves constructor projection before the call-only fallback and removes the chained-prototype path that forced TS7009. The older checker fixtures now assert their independently checked TypeScript 5.7.3 results.

Evidence: two focused checker controls passed (including the virtual chained-prototype fixture that previously returned TS7009), both focused Program chained-prototype tests passed (including number → string → removed → restored source revisions in both checkJs modes), guarded home-tsc built successfully, and the exact admitted-JavaScript CLI auditor remained 134/134. All builds/tests used the 3,840 MB guard; the home-tsc peak counted tree footprint was 2,206 MB.

The broader checker suite completed 4,705 pass / 11 fail / 4,716 total. The remaining failures are tracked in #968; they are not claimed as passing. The focused LSP package build was stopped by the 3,840 MB tree ceiling (exit 125, largest sampled process about 1,990 MB), so its edit/revert test remains unverified and local. The full Program suite also remains to run. #846 and #66 stay open.

5 days ago

Importer-side control added: import Main = require('./sub-functions.js'); const instance = new Main(10); const nested = new Main.Sub(); const value: number = nested.instance.t; TypeScript 5.7.3 reports no diagnostics for owner or importer. Home additionally reports TS7009 on new Main.Sub() (and the three owner diagnostics already listed). This confirms the nested constructor's public construct signature is missing, not only an owner-side this error. Fixture path will be packages/ts_cli/testdata/issue966-commonjs-constructor/.

Sign in to comment on this issue.