Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable no unchecked index access rule #6441

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/apps/dashboard/routes/users/parentalcontrol.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,12 @@ const UserParentalControl = () => {
for (let i = 0, length = allParentalRatings.length; i < length; i++) {
rating = allParentalRatings[i];

if (!rating) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we even get allParentalRatings with null|undefined elements?
I'm not a fan of checking everything (following some "pattern"), ignoring the fact that it will never happen. 🤷‍♂️ If this can happen, we silently ignore the invalid argument (the array with the invalid element), which can make it harder to find the error.


if (ratings.length) {
const lastRating = ratings[ratings.length - 1];

if (lastRating.Value === rating.Value) {
if (lastRating && lastRating.Value === rating.Value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have a rule to use an optional chain instead of foo && foo.bar?
foo && foo.bar is more portable though.

lastRating.Name += '/' + rating.Name;
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ function playAllFromHere(opts: PlayAllFromHereOptions) {
let startIndex;

for (let i = 0, length = items?.length; i < length; i++) {
if (!items[i]) continue;

if (items[i] === item) {
foundCard = true;
startIndex = i;
}

if (foundCard || !queue) {
ids.push(items[i].Id);
ids.push(items[i]?.Id);
Copy link
Contributor

Choose a reason for hiding this comment

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

The optional chain is redundant - items[i] is checked above. But again, can items have invalid elements?

}
}

Expand Down
14 changes: 6 additions & 8 deletions src/apps/experimental/routes/home.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,13 @@
}

return import(/* webpackChunkName: "[request]" */ `../../../controllers/${depends}`).then(({ default: ControllerFactory }) => {
let controller = tabControllers[index];
const controller = tabControllers[index];
if (controller) return controller;

if (!controller) {
const tabContent = element.current?.querySelector(".tabContent[data-index='" + index + "']");
controller = new ControllerFactory(tabContent, null);
tabControllers[index] = controller;
}

return controller;
const tabContent = element.current?.querySelector(".tabContent[data-index='" + index + "']");
const newController = new ControllerFactory(tabContent, null);
tabControllers[index] = newController;
return newController;
});
}, [ tabControllers ]);

Expand Down Expand Up @@ -123,7 +121,7 @@
currentTabController.onResume({});
}
(document.querySelector('.skinHeader') as HTMLDivElement).classList.add('noHomeButtonHeader');
}, [ initialTabIndex, mainTabsManager ]);

Check warning on line 124 in src/apps/experimental/routes/home.tsx

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️

React Hook useCallback has a missing dependency: 'setTitle'. Either include it or remove the dependency array

const onPause = useCallback(() => {
const currentTabController = tabController.current;
Expand Down
2 changes: 1 addition & 1 deletion src/apps/experimental/routes/homevideos/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const homevideosTabMapping: LibraryTabMapping = {

const HomeVideos: FC = () => {
const { libraryId, activeTab } = useCurrentTab();
const currentTab = homevideosTabMapping[activeTab];
const currentTab = homevideosTabMapping[activeTab] || photosTabContent;

return (
<Page
Expand Down
2 changes: 1 addition & 1 deletion src/apps/experimental/routes/livetv/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const liveTvTabMapping: LibraryTabMapping = {

const LiveTv: FC = () => {
const { libraryId, activeTab } = useCurrentTab();
const currentTab = liveTvTabMapping[activeTab];
const currentTab = liveTvTabMapping[activeTab] || programsTabContent;

return (
<Page
Expand Down
2 changes: 1 addition & 1 deletion src/apps/experimental/routes/movies/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const moviesTabMapping: LibraryTabMapping = {

const Movies: FC = () => {
const { libraryId, activeTab } = useCurrentTab();
const currentTab = moviesTabMapping[activeTab];
const currentTab = moviesTabMapping[activeTab] || moviesTabContent;

return (
<Page
Expand Down
2 changes: 1 addition & 1 deletion src/apps/experimental/routes/music/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const musicTabMapping: LibraryTabMapping = {

const Music: FC = () => {
const { libraryId, activeTab } = useCurrentTab();
const currentTab = musicTabMapping[activeTab];
const currentTab = musicTabMapping[activeTab] || albumsTabContent;

return (
<Page
Expand Down
2 changes: 1 addition & 1 deletion src/apps/experimental/routes/shows/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const tvShowsTabMapping: LibraryTabMapping = {

const Shows: FC = () => {
const { libraryId, activeTab } = useCurrentTab();
const currentTab = tvShowsTabMapping[activeTab];
const currentTab = tvShowsTabMapping[activeTab] || seriesTabContent;

return (
<Page
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
private mediaSegments: MediaSegmentDto[] = [];

private async fetchMediaSegments(api: Api, itemId: string, includeSegmentTypes: MediaSegmentType[]) {
// FIXME: Replace with SDK getMediaSegmentsApi function when available in stable

Check warning on line 28 in src/apps/stable/features/playback/utils/mediaSegmentManager.ts

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️

Unexpected 'fixme' comment: 'FIXME: Replace with SDK...'
const mediaSegmentsApi = new MediaSegmentsApi(api.configuration, undefined, api.axiosInstance);

try {
Expand Down Expand Up @@ -124,7 +124,7 @@
const currentSegmentDetails = findCurrentSegment(this.mediaSegments, time, this.lastSegmentIndex);
if (
// The current time falls within a segment
currentSegmentDetails
currentSegmentDetails?.segment
// and the last segment is not ignored or the segment index has changed
&& (!this.isLastSegmentIgnored || this.lastSegmentIndex !== currentSegmentDetails.index)
) {
Expand Down
12 changes: 6 additions & 6 deletions src/apps/stable/features/playback/utils/mediaSegments.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
import type { MediaSegmentDto } from '@jellyfin/sdk/lib/generated-client/models/media-segment-dto';

const isBeforeSegment = (segment: MediaSegmentDto, time: number, direction: number) => {
const isBeforeSegment = (segment: MediaSegmentDto | undefined, time: number, direction: number) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be better to force the caller to check that segment is not "null", instead of putting optioal chains everywhere?

if (direction === -1) {
return (
typeof segment.EndTicks !== 'undefined'
typeof segment?.EndTicks !== 'undefined'
&& segment.EndTicks <= time
);
}
return (
typeof segment.StartTicks !== 'undefined'
typeof segment?.StartTicks !== 'undefined'
&& segment.StartTicks > time
);
};

export const isInSegment = (segment: MediaSegmentDto, time: number) => (
typeof segment.StartTicks !== 'undefined'
export const isInSegment = (segment: MediaSegmentDto | undefined, time: number) => (
typeof segment?.StartTicks !== 'undefined'
&& segment.StartTicks <= time
&& (typeof segment.EndTicks === 'undefined' || segment.EndTicks > time)
);
Expand All @@ -26,7 +26,7 @@ export const findCurrentSegment = (segments: MediaSegmentDto[], time: number, la
}

let direction = 1;
if (lastIndex > 0 && lastSegment.StartTicks && lastSegment.StartTicks > time) {
if (lastIndex > 0 && lastSegment?.StartTicks && lastSegment.StartTicks > time) {
direction = -1;
}

Expand Down
4 changes: 4 additions & 0 deletions src/components/actionSheet/actionSheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ function getPosition(positionTo: Element, options: Options, dlg: HTMLElement) {

const pos = getOffsets([positionTo])[0];

if (!pos) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

The only case (with [positionTo]) where pos will be invalid is if document is not present. 😐


if (options.positionY !== 'top') {
pos.top += (pos.height || 0) / 2;
}
Expand Down Expand Up @@ -250,6 +252,8 @@ export function show(options: Options) {
for (let i = 0; i < options.items.length; i++) {
const item = options.items[i];

if (!item) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can happen?
If can - the error is shadowed.


if (item.divider) {
html += '<div class="actionsheetDivider"></div>';
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/components/cardbuilder/Card/cardHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ function getParentTitle(
serverId: NullableString,
item: ItemDto
) {
if (isOuterFooter && item.AlbumArtists?.length) {
if (isOuterFooter && item.AlbumArtists?.length && item.AlbumArtists[0]) {
(item.AlbumArtists[0] as ItemDto).Type = ItemKind.MusicArtist;
(item.AlbumArtists[0] as ItemDto).IsFolder = true;
return getTextActionButton(item.AlbumArtists[0], null, serverId);
Expand Down
4 changes: 2 additions & 2 deletions src/components/homesections/sections/libraryButtons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ function getLibraryButtonsHtml(items: BaseItemDto[]) {
// library card background images
for (let i = 0, length = items.length; i < length; i++) {
const item = items[i];
const icon = imageHelper.getLibraryIcon(item.CollectionType);
html += '<a is="emby-linkbutton" href="' + appRouter.getRouteUrl(item) + '" class="raised homeLibraryButton"><span class="material-icons homeLibraryIcon ' + icon + '" aria-hidden="true"></span><span class="homeLibraryText">' + escapeHtml(item.Name) + '</span></a>';
const icon = imageHelper.getLibraryIcon(item?.CollectionType);
html += '<a is="emby-linkbutton" href="' + appRouter.getRouteUrl(item) + '" class="raised homeLibraryButton"><span class="material-icons homeLibraryIcon ' + icon + '" aria-hidden="true"></span><span class="homeLibraryText">' + escapeHtml(item?.Name || '') + '</span></a>';
}

html += '</div>';
Expand Down
8 changes: 6 additions & 2 deletions src/components/listview/List/listHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ const sortBySortName = (item: ItemDto): string => {
}

// SortName
const name = (item.SortName ?? item.Name ?? '?')[0].toUpperCase();
const name = (item.SortName ?? item.Name ?? '?')[0]?.toUpperCase();

if (!name) return '';

const code = name.charCodeAt(0);
if (code < 65 || code > 90) {
Expand Down Expand Up @@ -48,7 +50,9 @@ const sortByAlbumArtist = (item: ItemDto): string => {
return '';
}

const name = item.AlbumArtist[0].toUpperCase();
const name = item.AlbumArtist[0]?.toUpperCase();

if (!name) return '';

const code = name.charCodeAt(0);
if (code < 65 || code > 90) {
Expand Down
31 changes: 18 additions & 13 deletions src/elements/emby-scrollbuttons/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ function getFirstAndLastVisible(scrollFrame: HTMLElement, items: HTMLElement[],

const currentScrollPos = scrollPosition * localeModifier;
const scrollerWidth = scrollFrame.offsetWidth;
const itemWidth = items[0].offsetWidth;
const itemWidth = items[0]?.offsetWidth || 1;

// Rounding down here will give us the first item index which is fully visible. We want the first partially visible
// index so we'll subtract one.
const firstVisibleIndex = Math.max(Math.floor(currentScrollPos / itemWidth) - 1, 0);
// Rounding up will give us the last index which is at least partially visible (overflows at container end).
const lastVisibleIndex = Math.floor((currentScrollPos + scrollerWidth) / itemWidth);

return [firstVisibleIndex, lastVisibleIndex];
return { firstVisibleIndex, lastVisibleIndex };
}

function scrollToWindow({
Expand All @@ -71,27 +71,32 @@ function scrollToWindow({
// factory functions on it, but is not a true scroller factory. For legacy, we need to pass `scroller` directly
// instead of getting the frame from the factory instance.
const frame = scroller.getScrollFrame?.() ?? scroller;
const [firstVisibleIndex, lastVisibleIndex] = getFirstAndLastVisible(frame, items, scrollState);
const { firstVisibleIndex, lastVisibleIndex } = getFirstAndLastVisible(frame, items, scrollState);

let scrollToPosition: number;
let scrollToPosition = 0;

if (direction === ScrollDirection.RIGHT) {
const nextItem = items[lastVisibleIndex];

if (nextItem) {
// This will be the position to anchor the item at `lastVisibleIndex` to the start of the view window.
const nextItemScrollOffset = lastVisibleIndex * nextItem.offsetWidth;
scrollToPosition = nextItemScrollOffset * localeModifier;
const nextItemScrollOffset = lastVisibleIndex * nextItem.offsetWidth;
scrollToPosition = nextItemScrollOffset * localeModifier;
}
} else {
const previousItem = items[firstVisibleIndex];
const previousItemScrollOffset = firstVisibleIndex * previousItem.offsetWidth;

// Find the total number of items that can fit in a view window and subtract one to account for item at
// `firstVisibleIndex`. The total width of these items is the amount that we need to adjust the scroll position by
// to anchor item at `firstVisibleIndex` to the end of the view window.
const offsetAdjustment = (Math.floor(frame.offsetWidth / previousItem.offsetWidth) - 1) * previousItem.offsetWidth;
if (previousItem) {
const previousItemScrollOffset = firstVisibleIndex * previousItem.offsetWidth;

// This will be the position to anchor the item at `firstVisibleIndex` to the end of the view window.
scrollToPosition = (previousItemScrollOffset - offsetAdjustment) * localeModifier;
// Find the total number of items that can fit in a view window and subtract one to account for item at
// `firstVisibleIndex`. The total width of these items is the amount that we need to adjust the scroll position by
// to anchor item at `firstVisibleIndex` to the end of the view window.
const offsetAdjustment = (Math.floor(frame.offsetWidth / previousItem.offsetWidth) - 1) * previousItem.offsetWidth;

// This will be the position to anchor the item at `firstVisibleIndex` to the end of the view window.
scrollToPosition = (previousItemScrollOffset - offsetAdjustment) * localeModifier;
}
}

if (scroller.slideTo) {
Expand Down
3 changes: 3 additions & 0 deletions src/utils/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ export function readFileAsBase64(file: File): Promise<string> {
reader.onload = (e) => {
// Split by a comma to remove the url: prefix
const data = (e.target?.result as string)?.split?.(',')[1];
if (!data) {
return reject(new Error('No file data'));
}
resolve(data);
};
reader.onerror = reject;
Expand Down
2 changes: 1 addition & 1 deletion src/utils/number.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,5 @@ export function decimalCount(value: number): number {

const arr = value.toString().split('.');

return arr[1].length;
return arr[1]?.length || 0;
}
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"skipLibCheck": true,
"esModuleInterop": true,
"noImplicitAny": true,
"noUncheckedIndexedAccess": true,
"allowSyntheticDefaultImports": true,
"strict": true,
"module": "ESNext",
Expand Down
Loading