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

window.fdc3 should be marked as optional #1385

Open
Tracked by #1391 ...
Roaders opened this issue Oct 12, 2024 · 4 comments · Fixed by #1386
Open
Tracked by #1391 ...

window.fdc3 should be marked as optional #1385

Roaders opened this issue Oct 12, 2024 · 4 comments · Fixed by #1386
Labels
api FDC3 API Working Group enhancement New feature or request FDC3 for Web Browsers needs-pr

Comments

@Roaders
Copy link
Contributor

Roaders commented Oct 12, 2024

Enhancement Request

Use Case:

I want to test to see if window.fdc3 is defined

For example:
In writing and testing a getAgent implementation it's important to see if an agent has already been created and assigned to window.fdc3. Currently the typing specified it as being DesktopAgent:

declare global {
  interface Window {
    fdc3: DesktopAgent;
  }
}

This however should be marked as optional so any code accessing that property check to see if it's defined first:

declare global {
  interface Window {
    fdc3?: DesktopAgent;
  }
}
@Roaders Roaders added the enhancement New feature or request label Oct 12, 2024
@Roaders
Copy link
Contributor Author

Roaders commented Oct 12, 2024

I realise that this is probably going to be an issue as anyone relying on this type being defined will now get a breaking change as it might not be defined. My proposal is IMHO the "right" thing to do but I understand that there will likely be issues and push back.

@kriswest
Copy link
Contributor

HI @Roaders - this is very good point that we haven't discussed in a long time. getAgent does set window.fdc3 by default (if not already set by a 'Desktop Agent Preload' agent, i.e. a traditional FDC3 DA) for backwards compatibility but can be prevented from doing so. Hence, it will not always be set and the typing (which you can't avoid using if importing the @finos/fdc3 lib) may be incorrect when that setting is used and during the set-up of the connection to a web-based DA. We'll need a PR into the https://github.com/finos/FDC3/tree/fdc3-for-web branch to make that change and close this issue - it's a significant (if tiny) change, but is what has been intended since early in the FDC3 for the Web's history.

If you have a moment, please feel free to raise that PR (which you can do at https://github.com/finos/FDC3/edit/fdc3-for-web/src/index.ts) and we'll check with others at the meeting on Thursday and SWG meeting the week after (as its a change to what was voted on).

@kriswest
Copy link
Contributor

kriswest commented Oct 15, 2024

P.S. PR must include a CHANGELOG.md entry describing that change.

@Roaders
Copy link
Contributor Author

Roaders commented Oct 15, 2024

Great! pr raised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api FDC3 API Working Group enhancement New feature or request FDC3 for Web Browsers needs-pr
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants