Conversation
1 of 3 tasks
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
3 tasks
Turns off no-explicit-any and the no-unsafe-* rules for specs and the test helpers that are kept out of the production bundle. Test doubles and expectation helpers routinely work with loosely typed values, and the typing effort under OP-20341 targets application code only. The two no-unsafe exemptions that lived in the vitest spec block move into the new block, which covers the helper files as well. Removes the inline disables in specs and test setup that the exemption makes redundant. https://community.openproject.org/wp/OP-20342
Removes the api.v3 Result, Collection and Duration interfaces, the Factory global and the op.QueryParams namespace. Nothing references them, and most declared their members as any. https://community.openproject.org/wp/OP-20342
Declares the tablesorter call and its language defaults, and the metadata plugin slot the reporting page clears, instead of any. The reporting controller no longer needs its inline disables for them. https://community.openproject.org/wp/OP-20342
Adds a HookSignatures map for the hooks the core registers and calls, so register and call check callback parameters and return typed results instead of any. Arbitrary hook names still fall back to an open signature, because plugins register and call hooks by name. The fallback excludes the known names, so a mismatched callback or argument for a known hook fails to compile; the spec pins that down. The bulk context menu actions are now declared as WorkPackageAction, which the typed hook result exposed as narrower than its use. https://community.openproject.org/wp/OP-20342
Types the dynamic component output maps with ng-dynamic-component's EventHandler, which is what ndcDynamicOutputs accepts, and the checksum callback as a plain thunk. The unconstrained Function type accepted any callable and let calls through unchecked. https://community.openproject.org/wp/OP-20342
Deletes WorkPackageResource#updateLinkedResources. It has no callers and calls an undeclared wpCacheService, so it only compiled because of the resource's `any` index signature. https://community.openproject.org/wp/OP-20344
Makes HalLink#$fetch and #$callable generic and gives CallableHalLink its call signature and a HalLink-typed $link, so link functions no longer rely on the global `Function.$link?:any` augmentation, which is removed. HalResource#$links is now a map of CallableHalLink and #$embedded a map of unknown values. Resource-specific link interfaces declare their action links as CallableHalLink<T> instead of `Promise<any>` methods, and are intersected with the base maps on `$links`/`$embedded` so the resource interfaces they are merged into keep the `any` index signature until it is flipped separately. WorkPackageResourceEmbedded drops its `HalResource|any` unions and the link interface no longer inherits the embedded attributes. The unused QueryFilterResource embedded declaration is removed. Callers that relied on the untyped maps get boundary casts. https://community.openproject.org/wp/OP-20344
Types HalResource#$source as HalSource and accepts an unknown source in the constructor and $initialize, which already unwrap a HalResource passed in place of a raw source. The initializer callback takes a HalResource, $copy and $plain drop their `any`, and lazy() becomes generic over the value it defines. initializeHalProperties now reads the source through typed views of `_links` and `_embedded` and passes the target maps to setupProperty directly instead of looking them up by a computed `$`-name, removing the builder's `any` casts. Runtime behaviour is unchanged. Callers and specs that relied on the untyped source get boundary casts. https://community.openproject.org/wp/OP-20344
Types ResourceChangeset#schema and WorkPackageChangeset#schema as ISchemaProxy, which is what both getters return. FormResource#schema stays the embedded form schema and gets its own FormSchemaResource type, a SchemaResource whose attributes are field schemas, and its commit link becomes a CallableHalLink. IFieldSchema is now derived from IOPFieldSchema instead of duplicating it with `any`-typed allowedValues and options. Fields that load allowed values through the link cast it at the boundary. SchemaProxy dispatches its proxied methods by name through proxyMethod, which keeps them bound to the handler and still honours subclass overrides, instead of wrapping unbound method references in a Function proxy. The payload helper checks own schema keys with Object.hasOwn. https://community.openproject.org/wp/OP-20344
Declares attributes and link callables that callers already read through the `any` index signature: FormResource#payload, #commit and #configureForm, QueryResource#star, #unstar, #icalUrl and #updatedAt, TimeEntryResource#hours, WorkPackageCollectionResource#customFields and #createWorkPackage, and WorkPackageResource#configureForm and #bcfViewpoints. WorkPackageResource#description becomes Formattable and RelationResource#type a string. Class and interface declaration merges are replaced by member declarations on the class (with `implements` for the exported shape), which emit no code under useDefineForClassFields: false. Resource state getters cast through unknown instead of `any`. Callers whose values are now typed get boundary casts. https://community.openproject.org/wp/OP-20344
Types the request data, created sources and registered classes in HalResourceService, the HTTP client options and interceptor, and ErrorResource's errors and details. HalResourceNotificationService takes ErrorResource where it reads error attributes and treats raw responses as unknown. HalError#resource is nullable, so the two callers passing it to showError assert it at the boundary, as the service already assumed. HTTPClientParamMap keeps its `any` values: query and BCF callers pass untyped objects as request params. https://community.openproject.org/wp/OP-20344
Types the remaining `any` members of the query filter, filter schema, schema dependency and grid resources, lets attachable resources read the now typed addAttachment link directly, and casts the two schema and parent reads that go through the resource index signature. https://community.openproject.org/wp/OP-20344
Removes type assertions, eslint-disable directives and imports that the typed HAL links, sources, schemas and resource attributes made unnecessary, and marks the two now typed $load() calls whose promises were already discarded with `void`. https://community.openproject.org/wp/OP-20344
Declares the base link map as holding a callable link, an array of links, or nothing, since the builder resolves array-valued HAL links to arrays and resources only carry the links the API sent. The self link is always present, as the builder creates one when missing. Call sites that read a link they have already checked, or that the resource always carries, cast it at the point of use. https://community.openproject.org/wp/OP-20344
Casts the resource attributes that display and edit fields read by name at the point of use, keeping today's values. These reads compiled only because HalResource declared an any index signature, which the next commit removes. https://community.openproject.org/wp/OP-20349
Casts work package attributes and raw links that views and services read without a declared type, keeping today's behaviour, ahead of the HalResource index signature becoming unknown. https://community.openproject.org/wp/OP-20349
Casts undeclared resource attributes read in grids, context menus, modals, search, BCF, the team planner and the costs and GitLab plugin modules, keeping today's behaviour, ahead of the HalResource index signature becoming unknown. https://community.openproject.org/wp/OP-20349
Casts the resource attribute a spec reads by name, since specs are typechecked even though they are exempt from the typing lint rules. https://community.openproject.org/wp/OP-20349
Declares the HalResource index signature as unknown instead of any. The any signature let every undeclared attribute read skip type checking, which a code comment had tracked as a source of bugs since 2019 (#31462). Reads of undeclared attributes now need a declaration or a cast, as the preceding commits add. The payload helper picks the current schema with ?? instead of ||; the value is a schema resource or nullish, so the result is unchanged. https://community.openproject.org/wp/OP-20349
Declares the two attribute group shapes the work package form schema embeds (attribute groups and query groups) and casts the schema's _attributeGroups to them at the boundary, so rebuilding the grouped fields no longer reads untyped members. Drops the redundant unknown intersection on the project casts. https://community.openproject.org/wp/OP-20347
Types the watchers tab's list as user resources, its autocompleter input as the user autocomplete item and its rejections as error resources. Types the aggregated activity pages as nested resource arrays and the full view's watchers as resource elements. https://community.openproject.org/wp/OP-20347
Types the change-parent payload, the children query props and the relation update params, and treats relation rejections as unknown, since they only flow into handleRawError, which accepts unknown. https://community.openproject.org/wp/OP-20347
Types the date-time filter parser and formatter inputs as the ISO strings they receive, and the single date-time filter's value getter as a string so its template binding matches. Types the query filter's operators as operator resources read from the filter schema and the unsubscribe notifier as an observable of unknown. https://community.openproject.org/wp/OP-20347
Types the query loading error as an error resource, the starred toggle as a promise of unknown, the query form schema as a query schema resource and the copy view's breadcrumbs as breadcrumb items, translating through the I18n service like its sibling entries. Makes the table edit form's requireVisible resolve to void, matching the EditForm contract whose only caller ignores the resolved container. Removes the unused types field from the create button. https://community.openproject.org/wp/OP-20347
Types the setValue overrides of the work package and time entry changesets as accepting unknown values and casts the time entry payload to its link bag before dropping the project link. https://community.openproject.org/wp/OP-20347
Types the view handler token as the handler registry it provides, dynamic component definitions with unknown inputs, timeline label entries as nullable strings and the custom error hook's argument as an error resource. Hoists the focus anchor tab label into a local so its type is not inferred as any from the translate options. https://community.openproject.org/wp/OP-20347
Types the API group value as a nullable string. The embedded graph now stringifies it explicitly for its count map key, which is what the computed key did implicitly, and collects labels as nullable strings. https://community.openproject.org/wp/OP-20347
myabc
force-pushed
the
implementation/op-20349-eslint-typing-fix-hal-index-signature
branch
from
September 29, 2026 09:47
ac71ea0 to
2705f0a
Compare
myabc
force-pushed
the
implementation/op-20347-eslint-typing-fix-wp-views
branch
from
September 29, 2026 09:47
8a55fcd to
ccf6f64
Compare
myabc
added
eslint
javascript
github-actions
Bot
added
the
ai: Directed 🪄
myabc
marked this pull request as ready for review
September 29, 2026 11:01
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Two newly introduced type contracts conflict with their actual API and dependency-injection values.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Types work package view data and service boundaries while aiming to preserve runtime behavior.
Changes:
- Adds types for form groups, watchers, activities, relations, filters, and grouped results.
- Replaces unsafe
anyusage with concrete orunknowntypes. - Aligns edit-form and routing APIs with existing consumers.
| File | Description |
|---|---|
wp-embedded-graph.component.ts |
Types grouped chart values. |
work-package-notification.service.ts |
Types custom API errors. |
work-package-single-view.base.ts |
Types translated tab labels. |
wp-view-timeline.service.ts |
Types timeline label entries. |
event-handler-registry.ts |
Changes handler token typing. |
wp-full-view.component.ts |
Types watcher collections. |
partitioned-query-space-page.component.ts |
Types dynamic components. |
time-entry-changeset.ts |
Types values and payload links. |
wp-single-view.component.ts |
Models form attribute groups. |
watchers-tab.component.ts |
Types watcher operations. |
wp-activity.service.ts |
Types activity collections. |
wp-relations.service.ts |
Types relation update parameters. |
wp-relations.interfaces.ts |
Removes relation-group any. |
wp-relations-hierarchy.service.ts |
Types parent-change payloads. |
wp-relations-hierarchy.directive.ts |
Types child query parameters. |
wp-relation-row.component.ts |
Types rejected errors. |
wp-relation-inline-add-existing.component.ts |
Types rejected errors. |
wp-states-initialization.service.ts |
Types query schemas. |
wp-list.service.ts |
Types errors and star operations. |
wp-inline-create.service.ts |
Types dynamic reference components. |
work-package-changeset.ts |
Types changeset values. |
table-edit-form.ts |
Aligns visibility return type. |
wp-copy-full-view.component.ts |
Types breadcrumb items. |
wp-create-button.component.ts |
Removes unused field. |
wp-breadcrumb-parent.component.ts |
Marks parent-change promise handled. |
wp-filters.service.ts |
Types unsubscribe observables. |
query-filter.component.ts |
Types available operators. |
filter-date-time-value.component.ts |
Types date-time values. |
abstract-filter-date-time-value.controller.ts |
Types date parser inputs. |
wp-collection-resource.ts |
Types grouped result values. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
|
|
||
| interface WorkPackageFormQueryGroup { | ||
| _type:'WorkPackageFormQueryGroup'; |
| } | ||
|
|
||
| export const WorkPackageViewHandlerToken = new InjectionToken<WorkPackageViewEventHandler<any>>('CardEventHandler'); | ||
| export const WorkPackageViewHandlerToken = new InjectionToken<WorkPackageViewHandlerRegistry<unknown>>('CardEventHandler'); |
myabc
force-pushed
the
implementation/op-20349-eslint-typing-fix-hal-index-signature
branch
from
September 29, 2026 18:29
2705f0a to
cdd9389
Compare
This branch has not been deployed
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.
Ticket
https://community.openproject.org/wp/OP-20347
What are you trying to accomplish?
Part of OP-20341, first of two pull requests for this ticket: removes ESLint typing violations from the work package views, without changing behaviour. The second pull request covers the work package table, fast table, card view and context menus.
setValueoverrides and view routing.Work package views go from 91 typing violations to 3; application-code typing violations drop from 531 to 434. The remaining 3 are on
WorkPackageChangeset#applyChanges, which #25662 types.What approach did you choose and why?
Changes are annotations and boundary casts; no narrowing guards were added. Form attribute groups are a union of the two group shapes the API returns. A few small, behaviour-preserving edits:
TableEditForm#requireVisiblenow returnsPromise<void>, as the abstractEditFormalready declares and its only caller expects; the copy view's breadcrumbs use the injectedI18nService, which wraps the same instance; an unused, uninitialisedtypesfield is removed.Typing grouped values as strings needed two small fixes in the embedded graph component, which keep the same keys and labels.
Stacked on #25661, which makes the
HalResourceindex signatureunknown. Independent of #25662 and #25663.AI involvement
Directed – I specified the requirements and AI implemented most of it; I validated via testing rather than a full line-by-line review.
Merge checklist
Browser JavaScript diff (5 files, type-only changes omitted)
Each changed browser file (TypeScript and templates, excluding specs, test helpers and declaration files) is transpiled on its own with
frontend/tsconfig.json, before and after; this is the emitted JavaScript that differs.