Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix(demo): Type errors #1976

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix(demo): Type errors #1976

wants to merge 2 commits into from

Conversation

literat
Copy link
Collaborator

@literat literat commented Mar 5, 2025

Description

Fixing type errors which are blocking web-twig demo builds.

Additional context

Issue reference

@github-actions github-actions bot added the bug Something isn't working label Mar 5, 2025
Copy link

netlify bot commented Mar 5, 2025

Deploy Preview for spirit-design-system-storybook canceled.

Name Link
🔨 Latest commit b50e049
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-storybook/deploys/67c86db0bd72630008e9c88d

Copy link

netlify bot commented Mar 5, 2025

Deploy Preview for spirit-design-system ready!

Name Link
🔨 Latest commit b50e049
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system/deploys/67c86db09b8ed000084f8d30
😎 Deploy Preview https://deploy-preview-1976--spirit-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 96 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 91 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@literat literat force-pushed the fix/spirit-web-type-errors branch 2 times, most recently from f57fdc2 to 0d2dcbb Compare March 5, 2025 13:26
@literat literat marked this pull request as ready for review March 5, 2025 13:26
@literat literat force-pushed the fix/spirit-web-type-errors branch from 0d2dcbb to 0b49361 Compare March 5, 2025 13:30
@literat literat requested a review from Copilot March 5, 2025 14:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This pull request aims to fix type errors that were blocking the web-twig demo builds by improving type annotations, adding missing type imports, and selectively suppressing type-checking errors. Key changes include:

  • Suppressing and bypassing type errors with ts-ignore/ts-expect-error comments in multiple files.
  • Adding type annotations and importing missing types (e.g. SpiritElement) to improve overall type safety.
  • Updating function calls (e.g. adding missing configuration objects) for better compatibility.

Reviewed Changes

File Description
packages/web/src/js/Toast.ts Suppresses a missing module error for 'csstype' and adds SpiritElement import.
apps/web-twig-demo/assets/scripts/form-validations.ts Introduces explicit conversion to integer values in validators to satisfy type checks.
packages/web/src/js/dom/InstanceMap.ts Adds a type import for SpiritElement to improve instance mapping.
apps/web-twig-demo/assets/scripts/file-uploader-image-preview.ts Refines type annotations for file objects and adds error suppression comments on type mismatches.
packages/web/src/js/Tooltip.ts Adds SpiritElement import and suppresses TS errors regarding missing properties on computed values.
apps/web-twig-demo/assets/scripts/file-uploader-meta-data.ts Updates type annotations in callbacks and suppresses TS errors related to DOM methods.
packages/web/src/js/FileUploader.ts Uses ts-ignore for FileList spread and suggests a safer conversion approach.
packages/web/src/js/Collapse.ts Suppresses type errors when iterating over HTMLCollection and hints at using a conversion to array.
apps/web-twig-demo/assets/app.ts Adjusts the FileUploader instance creation with an explicit empty configuration object.
packages/web/src/js/BaseComponent.ts Adds a ts-nocheck comment and a missing SpiritElement import for broader type compatibility.
packages/web/src/js/dom/Manipulator.ts, Password.ts, Modal.ts, Offcanvas.ts, Dropdown.ts, Tabs.ts, AutoResize.ts, ScrollView.ts Adds missing SpiritElement imports to address type errors.
apps/web-twig-demo/assets/scripts/toast-dynamic.ts Adds explicit type annotations for event handlers and suppresses errors on missing properties.

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

packages/web/src/js/FileUploader.ts:428

  • Rather than using @ts-ignore to spread target.files, consider converting the FileList to an array with Array.from(target.files) for clearer type handling.
// @ts-ignore -- TS2461: Type 'FileList' is not an array type.

@literat literat force-pushed the fix/spirit-web-type-errors branch from ab45257 to 8d64908 Compare March 5, 2025 14:54
@literat literat force-pushed the fix/spirit-web-type-errors branch from 8d64908 to b50e049 Compare March 5, 2025 15:28
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants