Deep dive for ICodeLoader

[scottn12]

Overview

The motivation to move ICodeLoader interface outside the container-definition is to separate abstractions layers. The @fluidframework/container-definition#ICodeLoader interface loads the package specified by code details and returns a promise to its entry point exports. @fluidframework/container-loader#ICodeDetailsLoader also performs the same functionality.

Affected Repositories:

Bohemia, Whiteboard, Fluid Framework

Files Affected

Fluid Framework:

1. common\lib\container-definitions\src\loader.ts
2. examples\hosts\iframe-host\src\inframehost.ts
3. packages\loader\container-loader\src\containerContext.ts
4. packages\loader\container-loader\src\loader.ts
5. packages\loader\web-code-loader\src\webLoader.ts
6. packages\test\test-utils\src\localCodeLoader.ts
7. packages\test\test-utils\src\localLoader.ts
8. packages\tools\replay-tool\src\replayLoaderObject.ts
9. server\routerlicious\packages\services\src\nodeCodeLoader.ts

Proposed Replacement

Remove ICodeLoader from @fluidframework/container-definitions. Replace ICodeLoader with ICodeDetailsLoader by importing it from @fluidframework/container-loader.

Issues

• Phase I: Move ICodeDetailsLoader and IFluidModuleWithDetails interface from @fluidframework/container-loader to @fluidframework/container-definition package. Keep the deprecated ICodeLoader interface #8480
• Phase II: Move @fluidframework/core-interface#fluidPackage.ts to @fluidframework/container-definition package #8485
• Remove drivers and driver defs dependencies on code loader types #9032
• Cleanup code loader types from @fluidframework/core-interfaces #9034
• Migrate partner repo to start using the ICodeDetailsLoader interface from @fluidframework/container-definition package. #8490

Next

• Move to using ICodeDetailsLoader while ingesting code loader types from container definitions #9033
• Remove ICodeLoader from FF repository #9657

[ChumpChief]

Re: Phase 1, I think probably the ICodeDetailsLoader interface and IFluidModuleWithDetails interface should be moved to the @fluidframework/container-definitions package, and then @fluidframework/container-loader and downstream customers can import from there instead. So that might be a Phase 0.

Now that I'm looking at it, I'm also thinking it would probably make more sense for basically all the stuff in fluidPackage.ts under @fluidframework/core-interfaces to move and be split between @fluidframework/container-definitions and @fluidframework/container-loader. Because code details, code loading, modules, etc. are not really "core" but rather specific to the container and its load flow. Currently these functionalities have their interfaces split between core-interfaces and container-definitions rather than a single package, and I don't understand why. If folks agree, now might be an opportune time to consolidate everything under container-definitions.

Also @anthony-murphy for visibility and his opinions.

[ChumpChief]

I see that fluidPackage.ts used to be in container-definitions until a little over a year ago with #3835 which moved it to core-interfaces, so it seems this move was intentional? But I still don't understand.

[anthony-murphy]

That seems fine. In retrospect that move looks like a mistake.

we are not there today, but i view our current common definitions as the place where we specify layer crossing contracts. The layers being: server, driver, host (loader), runtime, datastore, dds

The code loader is interesting, as it is not layer crossing itself. it is only a host concept, but we don't really have a great place to define and interface like that, as putting it in container-loader also brings all the implementations, making it hard for partners to structure their host code in a reusable way. I don't think that changes anything today, and container-definitions is probably the right place for it. But long term i could see leaf level definition packages that pair with implementation but include things that don't cross layers. The problem or course being package and version proliferation, but i think there are things we can do that ease that which i explore a bit in #6540.

[sonalivdeshpande]

That seems fine. In retrospect that move looks like a mistake.

we are not there today, but i view our current common definitions as the place where we specify layer crossing contracts. The layers being: server, driver, host (loader), runtime, datastore, dds

The code loader is interesting, as it is not layer crossing itself. it is only a host concept, but we don't really have a great place to define and interface like that, as putting it in container-loader also brings all the implementations, making it hard for partners to structure their host code in a reusable way. I don't think that changes anything today, and container-definitions is probably the right place for it. But long term i could see leaf level definition packages that pair with implementation but include things that don't cross layers. The problem or course being package and version proliferation, but i think there are things we can do that ease that which i explore a bit in #6540.

@anthony-murphy, if I have understood this correctly and container-definitions is the right place for ICodeLoader interface, then should the deprecated tag be removed from ICodeLoader? And why are we using the new interface @fluidframework/container-loader#ICodeDetailsLoader?

[ChumpChief]

ICodeLoader is still correctly deprecated and should still get removed -- but basically all the other code-loading-related interfaces should also move into container-definitions (including ICodeDetailsLoader, and all the stuff in fluidPackage.ts).

[sonalivdeshpande]

@anthony-murphy, @ChumpChief updated the Methodology to have four phases. Please take a look at it once. Thanks!

[ChumpChief]

Looks good to me!

[skylerjokiel]

Moved to ADO