Skip to content

chore: modernize lint and tsconfig setup#34

Open
zhannurkhan04 wants to merge 3 commits into
microsoft:mainfrom
zhannurkhan04:update-legacy-configs
Open

chore: modernize lint and tsconfig setup#34
zhannurkhan04 wants to merge 3 commits into
microsoft:mainfrom
zhannurkhan04:update-legacy-configs

Conversation

@zhannurkhan04

Copy link
Copy Markdown

No description provided.

Comment thread eslint.config.mjs
Comment thread package.json Outdated
Comment thread tsconfig.json Outdated
Comment thread package.json
Comment thread CHANGELOG.md Outdated
Comment thread tsconfig.json Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please verify formatting pane rendering against a real visual for these scenarios

The strict-mode refactor changes several displayName/description payloads that previously went to the host as undefined and now go as either a string fallback (card.name, "", formattingCard.displayName) or a localized value. The CHANGELOG mentions only that "SimpleCard top-level toggle placement" was preserved — but rendering can differ in ways that aren't covered by that. Could you build a sample visual against this branch, walk through each of the cases below, and confirm in a reply that the formatting pane looks identical to main?

  1. SimpleCard without displayNameKey / displayName — most exposed case; previously formattingGroup.displayName was undefined, now it equals card.name.
  2. SimpleCard with topLevelSlice (a ToggleSwitch in the card header).
  3. SimpleCard with multiple slices and no topLevelSlice.
  4. CompositeCard with multiple groups (each group's own displayName rendering).
  5. CompositeCard with a container (containerItems containing groups).
  6. Any card whose description was previously not set — now sent as "" instead of undefined.

A short reply confirming each item was checked is enough — no screenshots required.

Comment thread src/FormattingSettingsService.ts Outdated
Comment on lines +107 to +108
displayName: isSimpleCard ? formattingCard.displayName : getLocalizedProperty(cardGroupInstance, "displayName", this.localizationManager) ?? cardGroupInstance.name,
description: isSimpleCard ? "" : getLocalizedProperty(cardGroupInstance, "description", this.localizationManager) ?? "",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SimpleCard group displayName / description now duplicate the card title — looks like a behavior regression

Before:

displayName: isSimpleCard ? undefined : getLocalizedProperty(...),
description: isSimpleCard ? undefined : getLocalizedProperty(...),

After:

displayName: isSimpleCard ? formattingCard.displayName : ...,
description: isSimpleCard ? "" : ...,

For SimpleCards, formattingCard.displayName is the card's own title (line 72: localized value or card.name). Previously, passing undefined for the inner group's displayName was the signal Power BI uses to not render a separate group header inside a SimpleCard. With this change every SimpleCard will emit a group whose displayName matches the card's title, which can render a redundant group header above the slices.

The same concern applies to description: "" vs. undefined on the next line — empty string vs. absent value is a different wire payload for every existing SimpleCard.

If the intent was to preserve the old behavior, the simplest fix is to keep undefined for SimpleCards. Strict-mode can be satisfied by widening the local type or using a cast — example:

const formattingGroup: visuals.FormattingGroup = {
    displayName: (isSimpleCard ? undefined : (getLocalizedProperty(cardGroupInstance, "displayName", this.localizationManager) ?? cardGroupInstance.name)) as string,
    description: (isSimpleCard ? undefined : (getLocalizedProperty(cardGroupInstance, "description", this.localizationManager) ?? "")) as string,
    // ...
};

Worth covering with the manual checks requested in the file-level comment above before merging.

Comment thread CHANGELOG.md
### Changed
* Updated `powerbi-visuals-api` to `^5.11.0`.
* Enabled full TypeScript strict checks (`strictNullChecks`, `strictPropertyInitialization`, `noImplicitAny`) and fixed related type issues.
* Preserved formatting pane behavior for SimpleCard top-level toggle placement during strict-mode refactoring.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add an explicit ### Breaking section for the public API changes

This release contains at least two signature-level breaking changes that consumers of this package will hit when they upgrade. They are the primary justification for the 7.0.0 bump and should be called out explicitly so downstream visuals know to expect TS errors:

  1. getLocalizedProperty<T>(...) in src/utils/FormattingSettingsUtils.ts now returns string | undefined instead of string. Any code that does const name: string = getLocalizedProperty(...) will fail to compile.
  2. NamedEntity now declares [property: string]: unknown. Index access (entity["someKey"]) now returns unknown rather than any, which propagates to every subclass and may require explicit casts in consumer code.

Suggested addition near the top of the 7.0.0 entry:

### Breaking
* `getLocalizedProperty` now returns `string | undefined` instead of `string`; callers must handle the `undefined` case.
* `NamedEntity` now has an index signature `[property: string]: unknown` to support strict-mode property access; index access into any subclass now yields `unknown` instead of `any`.

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.

2 participants