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

refactor: Copy APIs out of common-definitions and remove dependencies #16638

Merged
merged 25 commits into from
Aug 17, 2023

Conversation

tylerbutler
Copy link
Member

@tylerbutler tylerbutler commented Aug 1, 2023

This PR deprecates the following types and interfaces from the common-definitions package:

  • interface IErrorEvent
  • interface IErrorEvent
  • interface IEvent
  • interface IEventProvider
  • type ExtendEventProvider
  • type IEventThisPlaceHolder
  • type IEventTransformer
  • type ReplaceIEventThisPlaceHolder

The code has been copied into the core-interfaces package for client use.

  • All client package uses have been updated to use the core-interfaces implementation instead of common-definitions.
  • There are no longer any client dependencies on common-definitions.
  • However, there are still server dependencies on common-definitions. It's not clear where the event-related APIs should live for server, so server continues to use the now-deprecated versions from common-definitions.
  • Also, protocol-definitions still imports and uses IErrorEvent and IEventProvider from common-definitions. I left those uses in place rather than copying the APIs into protocol-definitions.

@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: sharedstring area: dds: tree area: dev experience Improving the experience of devs building on top of fluid area: driver Driver related issues area: examples Changes that focus on our examples area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: loader Loader related issues area: odsp-driver area: runtime Runtime related issues area: server Server related issues (routerlicious) dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation public api change Changes to a public API base: next PRs targeted against next branch area: tests Tests to add, test infrastructure improvements, etc labels Aug 1, 2023
@tylerbutler tylerbutler marked this pull request as ready for review August 2, 2023 22:54
@tylerbutler tylerbutler requested review from msfluid-bot and a team as code owners August 2, 2023 22:54
@tylerbutler tylerbutler marked this pull request as draft August 11, 2023 23:22
@tylerbutler tylerbutler changed the base branch from next to main August 14, 2023 18:51
@github-actions github-actions bot added base: main PRs targeted against main branch and removed area: contributor experience area: server Server related issues (routerlicious) base: next PRs targeted against next branch labels Aug 14, 2023
@tylerbutler tylerbutler marked this pull request as ready for review August 14, 2023 19:19
@github-actions
Copy link
Contributor

🔗 Found some broken links! 💔

Run a link check locally to find them. See
https://github.com/microsoft/FluidFramework/wiki/Checking-for-broken-links-in-the-documentation for more information.

linkcheck output


> fluidframework-docs@0.25.0 ci:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start http://localhost:1313 linkcheck:full

1: starting server using command "npm run ci:start"
and when url "[ 'http://localhost:1313' ]" is responding with HTTP status code 200
running tests using command "npm run linkcheck:full"


> fluidframework-docs@0.25.0 ci:start
> http-server ./public --port 1313 --silent


> fluidframework-docs@0.25.0 linkcheck:full
> npm run linkcheck:fast -- --external


> fluidframework-docs@0.25.0 linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

http://localhost:1313/docs/apis/
- (956:8) '@fluid-i..' => http://localhost:1313/docs/apis/app-insights-logger (HTTP 404)

http://localhost:1313/docs/apis/runtime-definitions/
- (1786:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)
- (1786:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)

http://localhost:1313/docs/build/bundlers/
- (843:233) 'here' => https://github.com/microsoft/FluidFramework/blob/a4c38234a920abe9b54b1c26a14c0a8e430cd3fa/packages/tools/webpack-fluid-loader/webpack.config.js#L37 (HTTP 200 but missing anchor)

http://localhost:1313/docs/deployment/service-options/
- (848:72) 'Routerli..' => https://github.com/microsoft/FluidFramework/tree/main/server#readme (HTTP 200 but missing anchor)
- (850:168) 'Routerli..' => https://github.com/microsoft/FluidFramework/tree/main/server#readme (HTTP 200 but missing anchor)

http://localhost:1313/docs/faq/
- (927:87) 'Routerli..' => https://github.com/microsoft/FluidFramework/tree/main/server#readme (HTTP 200 but missing anchor)

http://localhost:1313/docs/testing/telemetry/
- (848:3) 'ILoaderP..' => https://github.com/microsoft/FluidFramework/blob/main/packages/loader/container-loader/src/loader.ts#L313 (HTTP 200 but missing anchor)


Summary of most serious issues:

http://localhost:1313/docs/apis/
- (956:8) '@fluid-i..' => http://localhost:1313/docs/apis/app-insights-logger (HTTP 404)

http://localhost:1313/docs/apis/runtime-definitions/
- (1786:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)
- (1786:37) 'https://..' => https://datatracker.ietf.org/doc/html/rfc4122%29 (HTTP 404)


Stats:
  204049 links
    1990 destination URLs
       2 URLs ignored
       5 warnings
       2 errors

 ELIFECYCLE  Command failed with exit code 1.

@tylerbutler tylerbutler merged commit a8c8150 into microsoft:main Aug 17, 2023
@tylerbutler tylerbutler deleted the cp-common-defs branch August 17, 2023 23:50
jatgarg pushed a commit to jatgarg/FluidFramework-1 that referenced this pull request Aug 18, 2023
…microsoft#16638)

This PR deprecates the following types and interfaces from the
common-definitions package:

-   interface IErrorEvent
-   interface IErrorEvent
-   interface IEvent
-   interface IEventProvider
-   type ExtendEventProvider
-   type IEventThisPlaceHolder
-   type IEventTransformer
-   type ReplaceIEventThisPlaceHolder

The code has been copied into the core-interfaces package for client
use.

- All client package uses have been updated to use the core-interfaces
implementation instead of common-definitions.
- There are no longer any client dependencies on common-definitions.
- **However,** there are still _server_ dependencies on
common-definitions. It's not clear where the event-related APIs should
live for server, so server continues to use the now-deprecated versions
from common-definitions.
- Also, protocol-definitions still imports and uses IErrorEvent and
IEventProvider from common-definitions. I left those uses in place
rather than copying the APIs into protocol-definitions.
vladsud added a commit that referenced this pull request Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: sharedstring area: dds: tree area: dds Issues related to distributed data structures area: dev experience Improving the experience of devs building on top of fluid area: driver Driver related issues area: examples Changes that focus on our examples area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: loader Loader related issues area: odsp-driver area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants