sachinchoolur / lightGallery

A customizable, modular, responsive, lightbox gallery plugin.
https://www.lightgalleryjs.com/
Other
6.42k stars 1.28k forks source link

typescript definitiion / description for `selector` option is invalid #1137

Open leevigraham opened 3 years ago

leevigraham commented 3 years ago

Description

Following the docs:

Based on your markup structure, you can specify custom selectors to fetch media data for the gallery. Pass "this" to select same element. You can also pass HTMLCollection directly Example - '.my-selector' | '#my-selector' | this | document.querySelectorAll('.my-selector')

document.querySelectorAll('.my-selector') returns a NodeListOf<Element> not HTMLCollection

Steps to reproduce

Create a simple gallery:

<div id="gallery">
  <div class="gallery-item" data-src="https://placeimg.com/1280/960/nature">
    <img src="https://placeimg.com/1280/960/nature" width="128" height="96">
  </div>
</div>

with the following typescript

import lightGallery from "lightgallery";
lightgallery(document.getElementById('gallery'), {
  selector: document.querSelectorAll('.gallery-item');
});

Expected result

Gallery opens

Actual result

Typescript Compilation error:

[tsl] ERROR in /usr/local/var/Sites/flotespace/theme/frontend/scripts/stimulus/gallery-controller.ts(19,47)
      TS2345: Argument of type '{ plugins: never[]; speed: number; selector: NodeListOf<Element>; }' is not assignable to parameter of type 'Partial<LightGalleryAllSettings>'.
  Types of property 'selector' are incompatible.
    Type 'NodeListOf<Element>' is not assignable to type 'string | HTMLCollection[] | undefined'.
      Type 'NodeListOf<Element>' is missing the following properties from type 'HTMLCollection[]': pop, push, concat, join, and 24 more.
sachinchoolur commented 3 years ago

Hey @leevigraham,

Thank you for reporting this issue. Yes, this needs to be improved. For now, please cast the selector as HTMLCollection[].

import lightGallery from "lightgallery";
lightgallery(document.getElementById('gallery'), {
  selector: document.querSelectorAll('.gallery-item') as HTMLCollection[]
});

Also, unless there is any specific requirement, you can just pass the '.gallery-item' as selector

import lightGallery from "lightgallery";
lightgallery(document.getElementById('gallery'), {
  selector: '.gallery-item'
});
leevigraham commented 3 years ago

Hi @sachinchoolur,

Thanks for the super fast reply :)

Unfortunately you cannot cask to HTMLCollection[]

ERROR in /usr/local/var/Sites/flotespace/theme/frontend/scripts/stimulus/gallery-controller.ts
./theme/frontend/scripts/stimulus/gallery-controller.ts 12:25-86
[tsl] ERROR in /usr/local/var/Sites/flotespace/theme/frontend/scripts/stimulus/gallery-controller.ts(12,26)
      TS2352: Conversion of type 'NodeListOf<Element>' to type 'HTMLCollection[]' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'NodeListOf<Element>' is missing the following properties from type 'HTMLCollection[]': pop, push, concat, join, and 24 more.
leevigraham commented 3 years ago

Also the reason I'm not using a selector is because I'm writing a stimulusjs controller wrapper for lightGallery which has a slightly different setup. I would prefer to use a NodeListOf<Element> but I can work around it.

leevigraham commented 3 years ago

Also, unless there is any specific requirement, you can just pass the '.gallery-item' as selector

That was just me simplifying the example :)

sachinchoolur commented 3 years ago
import lightGallery from "lightgallery";
lightgallery(document.getElementById('gallery'), {
  selector: (document.querSelectorAll('.gallery-item') as unknown) as HTMLCollection[]
});

Consider this as a temporary fix, once you upgrade to the newer version, you can remove the type casting

leevigraham commented 3 years ago

It ended up being even easier in the stimulus controller:

import { Controller } from "stimulus";

import "lightgallery/css/lightgallery.css";
import lightGallery from "lightgallery";

export default class extends Controller {
  static targets = ["galleryItem"];
  declare galleryItemTargets: HTMLCollection[];
  connect() {
    const options = {
      plugins: [],
      speed: 0,
      selector: this.galleryItemTargets,
    };
    lightGallery(this.element as HTMLElement, options);
  }
}

I have to declare some internal variables that are created at constructions so I just cast that as a HTMLCollection[]