Skip to content

Conversation

@NalinDalal
Copy link

Fixes #issue-number

Changes:
migrated client/modules/Preview to typescript

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123
  • meets the standards outlined in the accessibility guidelines

@release-com
Copy link

release-com bot commented Dec 5, 2025

Release Environments

This Environment is provided by Release, learn more!
To see the status of the Environment click on Environment Status below.

🔧Environment Status : https://app.release.com/public/Processing%20Foundation/env-3ead4d2e19

@clairep94
Copy link
Collaborator

Hi @NalinDalal, I've just run the CICD checks, and I think you have a few type-errors to resolve

Thanks for keeping a clean commit history! I see that all your commits have "no-verify" in the message, so I'm assuming you added the --no-verify flag to each?

Adding the --no-verify flag is only for when you have performed the initial update of the file extension from .js or .jsx to .ts or .tsx via git mv <file_relative_path>.js <file_relative_path>.ts

This is because we are expecting type errors when we perform this action, and we just want to commit the file extension update while conserving the file history

After this, please either run typechecks while you are working or allow the automated typecheck on commits to run by commit as usual without the --no-verify flag

@NalinDalal
Copy link
Author

yes, i noticed there are type-errors after I messaged on discord, will update soon

@clairep94 clairep94 added the Area: Typescript Migration Related to Typescript Migration after the initial pr05 grant 2025 migration work label Dec 5, 2025
@NalinDalal
Copy link
Author

uhh, I made the changes, yes, thing is I think I took your tutorial literally, yes I've ran type-checks locally, now seems to pass, added the image for preview
Screenshot 2025-12-05 at 8 44 43 AM

@nbogie
Copy link

nbogie commented Dec 5, 2025

Hi! I note there are types available for jshint and decomment from DefinitelyTyped:

@NalinDalal
Copy link
Author

NalinDalal commented Dec 6, 2025

Hi! I note there are types available for jshint and decomment from DefinitelyTyped:

@nbogie , Hi I am not quite sure I follow, can you explain?
are you asking me to include types for jshint and decomment?

@nbogie
Copy link

nbogie commented Dec 7, 2025

Hi @NalinDalal ! Yes, including those types (as dev dependencies) would allow typescript to spot any type-errors in the codebase's use of those libraries, as well as give intellisense and (possibly) inline docs for those usages, too.

At the moment, your PR declares those library variables in a way which means they'll be given type any, essentially disabling type-checking for calls to those libraries and for the subsequent use of return values from those calls.

Just a suggestion!

@NalinDalal
Copy link
Author

Hi @NalinDalal ! Yes, including those types (as dev dependencies) would allow typescript to spot any type-errors in the codebase's use of those libraries, as well as give intellisense and (possibly) inline docs for those usages, too.

At the moment, your PR declares those library variables in a way which means they'll be given type any, essentially disabling type-checking for calls to those libraries and for the subsequent use of return values from those calls.

Just a suggestion!

thanks for heads up, types added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Typescript Migration Related to Typescript Migration after the initial pr05 grant 2025 migration work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants