8000 Refactored fcl-core normalizers folder files to TypeScript by mfbz · Pull Request #2475 · onflow/fcl-js · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Refactored fcl-core normalizers folder files to TypeScript #2475

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

Merged
merged 10 commits into from
Jun 23, 2025

Conversation

mfbz
Copy link
Contributor
@mfbz mfbz commented May 27, 2025

Closes #2449

@mfbz mfbz requested a review from a team as a code owner May 27, 2025 21:50
Copy link
changeset-bot bot commented May 27, 2025

⚠️ No Changeset found

Latest commit: dcf41cc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mfbz mfbz changed the title Refactored fcl-core inormalizers folder files to TypeScript Refactored fcl-core normalizers folder files to TypeScript May 28, 2025

export function normalizeServices(services, data) {
export function normalizeServices(services: Service[], data?: any): any[] {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this return Service[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented 👍

@@ -28,9 +29,9 @@ const serviceNormalizers = {
"authn-refresh": normalizeAuthnRefresh,
}

export function normalizeService(service, data) {
export function normalizeService(service: Service, data?: any): any {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe same question here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented 👍

@chasefleming chasefleming requested a review from Copilot June 23, 2025 16:22
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.

Pull Request Overview

This PR refactors the entire normalizers/service folder from JavaScript to TypeScript, adding interfaces and type annotations, and removes the old .js files. It also tightens null-safety in two polling strategies.

  • Converted all service normalizers (*.js) to fully typed TypeScript (*.ts) with interfaces.
  • Updated normalizeServices and normalizeService with explicit parameter and return types.
  • Added optional chaining in poll.ts and http-post.ts to guard against null responses.

Reviewed Changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/fcl-core/src/normalizers/service/user-signature.ts New TypeScript version of user-signature normalizer
packages/fcl-core/src/normalizers/service/user-signature.js Removed legacy JS normalizer
packages/fcl-core/src/normalizers/service/service.ts Added types to normalizeServices / normalizeService
packages/fcl-core/src/normalizers/service/service.js Removed legacy JS wrapper
packages/fcl-core/src/normalizers/service/pre-authz.ts New TS version of pre-authz normalizer
packages/fcl-core/src/normalizers/service/pre-authz.js Removed legacy JS normalizer
packages/fcl-core/src/normalizers/service/polling-response.ts New TS version of polling-response normalizer
packages/fcl-core/src/normalizers/service/polling-response.js Removed legacy JS normalizer
packages/fcl-core/src/normalizers/service/open-id.ts New TS version of open-id normalizer
packages/fcl-core/src/normalizers/service/open-id.js Removed legacy JS normalizer
packages/fcl-core/src/normalizers/service/local-view.ts Updated TS local-view 10000 normalizer with docstrings
packages/fcl-core/src/normalizers/service/frame.ts New TS version of frame normalizer
packages/fcl-core/src/normalizers/service/composite-signature.ts New TS composite-signature normalizer
packages/fcl-core/src/normalizers/service/back-channel-rpc.ts New TS back-channel-rpc normalizer
packages/fcl-core/src/normalizers/service/authz.ts New TS authz normalizer with interface
packages/fcl-core/src/normalizers/service/authn.ts New TS authn normalizer with interface
packages/fcl-core/src/normalizers/service/authn-refresh.ts New TS authn-refresh normalizer
packages/fcl-core/src/normalizers/service/account-proof.ts New TS account-proof normalizer
packages/fcl-core/src/current-user/exec-service/strategies/utils/poll.ts Added optional chaining on resp in poll loop
packages/fcl-core/src/current-user/exec-service/strategies/http-post.ts Added optional chaining on resp in HTTP-POST execution

...SERVICE_PRAGMA,
type: service.type,
uid: service.id,
endpoint: service.authn,
Copy link
Preview
Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

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

The endpoint field is being assigned from service.authn, but the input object uses endpoint. It should be endpoint: service.endpoint.

Suggested change
endpoint: service.authn,
endpoint: service.endpoint,

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was endpoint: service.authn in the js file, so I left it as it was for backward compatibility!


export function normalizeServices(services, data) {
export function normalizeServices(services: Service[], data?: any): any[] {
Copy link
Preview
Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

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

[nitpick] The return type is any[], but after filtering it always returns an array of normalized services. Consider using a more specific type like Service[] or a union of possible service interfaces.

Suggested change
export function normalizeServices(services: Service[], data?: any): any[] {
export function normalizeServices(services: Service[], data?: any): Service[] {

Copilot uses AI. Check for mistakes.

Copy lin 8000 k
Contributor Author

Choose a reason for hiding this comment

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

Implemented 👍

@mfbz mfbz requested a review from chasefleming June 23, 2025 19:48
Copy link
Contributor
@jribbink jribbink left a comment

Choose a reason for hiding this comment

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

This one LGTM, just make sure you test it with Blocto or FCL Dev Wallet as well as Flow Wallet (some of these functions are only used by particular wallet types)

@mfbz
Copy link
Contributor Author
mfbz commented Jun 23, 2025

Yes, always testing with https://github.com/onflow/fcl-next-harness all wallet interactions 💪

@mfbz mfbz merged commit f31227b into feature/typescript-fcl-core Jun 23, 2025
1 check passed
@mfbz mfbz deleted the mfbz/refactor-fcl-core-normalizers branch June 23, 2025 20:27
mfbz added a commit that referenced this pull request Jun 25, 2025
* Added core types export (#2437)

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Refactored fcl-core current-user folder files to TypeScript (#2454)

* Refactored current-user folder with corresponding type definitions

* Renamed CurrentUserFull to CurrentUserService

* Improved CurrentUser interface

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Refactored root files (#2479)

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Refactored serialize folder (#2476)

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Refactored fcl-core document folder files to TypeScript (#2464)

* Refactored document folder

* Update packages/fcl-core/src/document/document.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>
Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Refactored fcl-core utils folder files to TypeScript (#2477)

* Refactored utils folder

* Update packages/fcl-core/src/utils/chain-id/chain-id-watcher.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/utils/chain-id/chain-id-watcher.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Fixed minor typo

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>
Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Refactored fcl-core events folder files to TypeScript  (#2472)

* Refactored legacy events file

* Fixed for compatibility

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Refactored fcl-core app utils folder files to TypeScript (#2442)

* Refactored app-utils folder

* Added new types export

* Update packages/fcl-core/src/fcl-core.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Cleaned up any usage

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>
Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Refactored fcl-core exec folder files to TypeScript (#2473)

* Completed exec folder refactoring

* Substituted correct current-user usage

* Improved mutate options interface

* Cleanup any usage

* Refactored any usage to only when needed

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Refactored fcl-core discovery folder files to TypeScript (#2462)

* Refactored discovery folder

* Reused ActorContext from util-actor package

* Minor fix

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Refactored fcl-core wallet-utils folder files to TypeScript (#2478)

* Refactored wallet-utils folder

* Made account proof data optional

* Update packages/fcl-core/src/wallet-utils/encode-account-proof.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/encode-account-proof.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/encode-account-proof.ts

Co-authored-by: 
8C95
Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Improved sendMsgToFCL msg param

* Update packages/fcl-core/src/wallet-utils/send-msg-to-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/CompositeSignature.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/on-message-from-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/send-msg-to-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/send-msg-to-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/send-msg-to-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/send-msg-to-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/send-msg-to-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/wallet-utils/send-msg-to-fcl.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Removed useless casts

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>
Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Fixed connection not working

* Refactored fcl-core interaction-template-utils folder files to TypeScript (#2474)

* Completed refactoring of interaction-template-utils folder

* Update packages/fcl-core/src/interaction-template-utils/generate-template-id/generate-template-id-1.1.0.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Improved dependency pin test

* Fixed console warn spy on tests

* Added suggestion

* Update packages/fcl-core/src/interaction-template-utils/generate-template-id/generate-template-id-1.0.0.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Update packages/fcl-core/src/interaction-template-utils/generate-template-id/generate-template-id.ts

Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>
Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>

* Refactored fcl-core normalizers folder files to TypeScript (#2475)

* Refactored normalizers folder

* Fixed post merging issues

* Updated comments

* Updated service normalizers

* Update packages/fcl-core/src/normalizers/service/open-id.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Big documentation update (#2532)

Co-authored-by: mfbz <mfbz@users.noreply.github.com>

* Added changeset

---------

Co-authored-by: mfbz <mfbz@users.noreply.github.com>
Co-authored-by: Jordan Ribbink <17958158+jribbink@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

3 participants
0