Breeze / breeze-client

Breeze for JavaScript clients
MIT License
38 stars 17 forks source link

Migration from 1.x Documentation Incomplete #45

Open Prinsn opened 3 years ago

Prinsn commented 3 years ago

I'm currently in the process of migrating from Net Core 3.x to 5.0 and while it seems my backend is fully functional, my frontend (which, to be fair, is inherited) is having some problems.

My presumption is that by upgrading the BreezePersistanceManager and not the Breezeclient might lead to a problem, and so I am trying to upgrade the breeze-client.

However, the documentation appears to be incomplete as to the degree of changes, one of the most obvious ones is that the IProperty interface no longer exists, which seems like a pretty huge migration for anything that extends or uses this to not have a major replacement.

Further, the package itself fails compilation in Angular 7 (trying to upgrade one thing at a time, not sure if this is a failure point) giving

    ERROR in node_modules/breeze-client/src/entity-metadata.d.ts(936,9): error TS1086: An accessor cannot be declared in an ambient context.

So it is unclear how someone should mitigate these things when migrating.

Sample snippet that is giving a majority of the problems

//Original code being migrated without modification just as an example of where a singular point of difficulty lies
import { Entity, EntityType, IProperty, EntityAspect, EntityStateSymbol, MetadataStore, PropertyChangedEventArgs, EntityError, HttpResponse } from "breeze-client";

export interface MetaEntityType extends EntityType {
    displayName: string;
    meta: MetaInfo;
    props: {};
}

export interface MetaProperty extends IProperty {
    displayName: string;
    meta: MetaInfo;
}

//...
import { BreezeValidatorsService } from '../breeze-validators/breeze-validators.service';
import DefaultBreezeValidators from '../breeze-validators/default-breeze-validators';
import * as breeze from 'breeze-client';
import 'breeze-client-labs/breeze.savequeuing';

    private parseAnnotatedMetadata(data: Array<AnnotatedMetadata>) {
        const entityManager = this.masterManager;
        const metadataStore = entityManager.metadataStore;

        data.forEach((metaEntity: AnnotatedMetadata) => {
            const entityType = <MetaEntityType>metadataStore.getEntityType(metaEntity.key, true);

            if (entityType) {
                metadataStore[entityType.shortName] = function () {
                    return entityType;
                };

                entityType.displayName = metaEntity.value.meta.displayName;
                entityType.meta = metaEntity.value.meta;
                entityType.props = {};

                const entityProps = <MetaProperty[]>entityType.getProperties();
                entityProps.forEach((entityProp: MetaProperty) => {
                    entityType.props[entityProp.name] = function () {
                        return entityProp;
                    };

                    const metaProp = metaEntity.value.properties[entityProp.name];

                    if (metaProp) {
                        entityProp.displayName = metaProp.displayName;
                        entityProp.meta = metaProp;

                        DefaultBreezeValidators.setupEFValidators(entityProp.validators, entityProp.meta);
                    }
                }, this);
            }
        });

        this.breezeValidators.setupCustomValidators(data, entityManager);
    }
steveschmitt commented 3 years ago

Sorry about the lack of upgrade documentation. What version are you converting from and to?

Prinsn commented 3 years ago

breeze-client is at 1.7.1

Also we currently use the angular bridge at 1.1.0, but I'm kind of blindly trying to just get things to work as I am not the original maintainer on the project.

There's also a breeze-client-labs at latest, but I'm uncertain if we're actually using it for anything.

There's no specific need to update, it was just a thought that it would have better compatibility with .NET Core 5.0, so was targeting latest.

On Thu, Apr 8, 2021 at 1:53 PM Steve Schmitt @.***> wrote:

Sorry about the lack of upgrade documentation. What version are you converting from and to?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Breeze/breeze-client/issues/45#issuecomment-816021343, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYR7G547T472PR57ZOBKCTTHXURPANCNFSM42TIDWAQ .

-- Jeff Miller

steveschmitt commented 3 years ago

Breeze is currently compiled using TypeScript 3.8, but Angular 7 uses TypeScript 3.1, which may not be compatible with everything that is in the .d.ts files produced by 3.8.

I'll see if I can downlevel the dts files to be compatible.

Prinsn commented 3 years ago

That might not be necessary. We are trying to just move everything to current right now, so if that's the failure point, it should resolve itself.

Targetting Angular 11 as the goal, but that is in work.

Normal migration steps should be fine? Per the other thread, I dont know that migrating the client will fix the issue of incompatibility with persistence core?

On Thu, Apr 8, 2021, 4:52 PM Steve Schmitt @.***> wrote:

Breeze is currently compiled using TypeScript 3.8, but Angular 7 uses TypeScript 3.1, which may not be compatible with everything that is in the .d.ts files produced by 3.8.

I'll see if I can downlevel the dts files to be compatible.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Breeze/breeze-client/issues/45#issuecomment-816185448, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYR7G3N4C4HSL5OIQRBG6LTHYJSBANCNFSM42TIDWAQ .

Prinsn commented 3 years ago

I've completed the Angular upgrade to 9 and I'm still running into a spot of difficulty here

The two things I'm sitting on at the moment are what IProperty got migrated from

import { IProperty, } from "breeze-client";

export interface MetaProperty extends IProperty {
    displayName: string;
    meta: MetaInfo;
}

And how to migrate

import { EntityManager } from 'breeze-client/src/entity-manager';
import { DataType, EntityQuery, MetadataStore, SaveOptions } from 'breeze-client';

    private masterManager: breeze.EntityManager = new EntityManager({
        serviceName: this.serviceName,
        saveOptions: new breeze.SaveOptions({
            allowConcurrentSaves: true
        })
    });

    private cacheLookups() {
        const lookups = this.masterManager.exportEntities()
        this.store.save(Config.stateKeys.lookups, lookups);
        this.store.save(Config.stateKeys.lookupsHash, Config.hashes.currentLookupsHash);
    }

the this.masterManager.exportEntities() now appears to expect 2 arguments, so it's unclear as to what would now be required from things that did not exist.

Also, looking at:

constructor(http: HttpClient) {
    // the order is important
    ModelLibraryBackingStoreAdapter.register();
    UriBuilderODataAdapter.register();
    AjaxHttpClientAdapter.register(http);
    DataServiceWebApiAdapter.register();
}

the upgrade step is unclear, as the first, second, and fourth do not appear to have automatic suggested imports, so it's confusing as to whether or not this requires further libraries or not?

Also

entityType.createEntity();

Now has a mandatory argument for initial values. How is this expected to be used, is null fine or does it require an empty object to maintain parity of function?

Lastly, I'm getting some compilation errors related to what might ultimately just by Ivy compiler changes with Angular 9 which are unclear as to how one is expected to use them

src/app/services/repository.ts:123:37 - error TS2339: Property 'createEntity' does not exist on type 'EntityManager'.

123         const e = <T>this.manager().createEntity(this.entityTypeName, config);
                                        ~~~~~~~~~~~~
src/app/services/repository.ts:137:38 - error TS2339: Property 'addEntity' does not exist on type 'EntityManager'.

137         const added = this.manager().addEntity(entity);
                                         ~~~~~~~~~
src/app/services/repository.ts:163:14 - error TS2339: Property 'saveChanges' does not exist on type 'EntityManager'.

163             .saveChanges(modified)
                 ~~~~~~~~~~~
src/app/services/repository.ts:226:37 - error TS2339: Property 'getEntityByKey' does not exist on type 'EntityManager'.

226             let entity = <T>manager.getEntityByKey(key);
                                        ~~~~~~~~~~~~~~
src/app/services/repository.ts:233:25 - error TS2339: Property 'attachEntity' does not exist on type 'EntityManager'.

233                 manager.attachEntity(entity);

In code, it appears that it recognizes that these exist, VS F12 takes me to metadata.d.ts and it works fine, but I had to manually include our own typing file, but since that typing file is within a node module, and this problem hasn't existed in any other library, I'm not sure how to procede.

This appears to be the totality of issues blocking upgrading the breeze client from 1.7 to 2.0, prior to actually testing the code.

Prinsn commented 3 years ago

I keep digging because I'm trying to see if I can solve any of this myself, and I've somehow gotten back around to

removed old things as upgrading to 2.0.11 fixed

It seems that all I'm left with are the following with respect to compilation.

New arguments for created entity

//...
        const entityType = <EntityType>this.getMetastore().getEntityType(this.entityTypeName);
        const manager = this.manager();

        return data.results.map((dto) => {
            const id = dto.id;
            const key = new EntityKey(entityType, id);
            let entity = <T>manager.getEntityByKey(key);
            if (!entity) {
               //What is the expected use of createEntity():  Arguments are now required and not optional
                entity = <T>entityType.createEntity();
//...

Migration of exportEntities to new required arguments

    private cacheLookups() {
        const lookups = this.masterManager.exportEntities();
        this.store.save(Config.stateKeys.lookups, lookups);
        this.store.save(Config.stateKeys.lookupsHash, Config.hashes.currentLookupsHash);
    }

 private buildManager(): void {
        this._manager = this.masterManager.createEmptyCopy();
        this._manager.saveOptions.allowConcurrentSaves = true;
        this._manager.enableSaveQueuing(true);

        // Populate with lookup data
        this._manager.importEntities(this.masterManager.exportEntities());
    }

Handling new return type from exportEntities(entities[], config)

    static cloneBreezeEntity(item: MetaEntity) {
        const manager = item.entityAspect.entityManager;
        // export w/o metadata and then parse the exported string.
        //This line no longer works on compile because exportEntities returns  an Object
        var exported = JSON.parse(manager.exportEntities([item], false));
        // extract the entity from the export
        var type = item.entityType;
        var copy = exported.entityGroupMap[type.name].entities[0];
        // remove the entityAspect
        delete copy.entityAspect;
        // remove the key properties
        type.keyProperties.forEach(function (p) { delete copy[p.name]; });

        // the "copy" provides the initial values for the create
        var newItem = manager.createEntity(type, copy) as MetaEntity;

        newItem.originalId = item.id;

        return newItem;
    }

the JSON.parse(manager.exportEntities([item], false)); no longer functions because the function returns an untyped Object, so it's unclear what the conversion of this code is.

And then I think that this project was originally using save-queuing from breeze-client-labs

//breeze.savequeuing.ts
import { EntityManager } from '../breeze-client';

declare module '../breeze-client' {
    interface EntityManager {
        enableSaveQueuing(enable?: boolean);
    }
}

Is this feature similarly available or native to the 2.0 client, or how is that to be migrated?

And lastly, I don't understand the documentation for

constructor(http: HttpClient) {
    // the order is important
    ModelLibraryBackingStoreAdapter.register(); //No default import
    UriBuilderODataAdapter.register(); //No default import
    AjaxHttpClientAdapter.register(http);
    DataServiceWebApiAdapter.register(); //No default import
}
steveschmitt commented 3 years ago

The IProperty interface is now called EntityProperty. It's a union of DataProperty and NavigationProperty classes; you can cast to one of those classes also.

The argument for entityType.createEntity() is optional; it's an object containing the initial values for the properties of the entity. I don't know why the argument appears to be required for you.

The arguments for entityManager.exportEntities are also optional. The first argument, if provided, is an array of the entities to export. The second argument is an object, such as { includeMetadata: false, asString: false } which controls whether to export the metadata, and whether to stringify the result. With no arguments, the behavior is to export all the entities in the entityManager, to include the metadata, and to export as a string. You can see the API docs here

For enableSaveQueuing, you just need the new import path:

import { enableSaveQueuing } from 'breeze-client/mixin-save-queuing';
...
  enableSaveQueuing(manager, true);

The UPGRADE document has an example of the imports for the adapters:

import { DataServiceWebApiAdapter } from 'breeze-client/adapter-data-service-webapi';
import { ModelLibraryBackingStoreAdapter } from 'breeze-client/adapter-model-library-backing-store';
import { UriBuilderODataAdapter } from 'breeze-client/adapter-uri-builder-odata';
import { AjaxHttpClientAdapter } from 'breeze-client/adapter-ajax-httpclient';

You can also see the example in the northwind-demo repo.

Prinsn commented 3 years ago

Awesome, thank you for that feedback.

The compiler is indicating that those properties are not optional, I'll double check the source library to double check if it's some goof.

Is there any benefit to migrating away from the angular2 bridge if it's currently working, or is it otherwise recommended to make the move to those types?

I'm not sure why it was giving me guff with extending DataProperty from home, but I'll double check on that. I dug into it and tried working with the types directly, maybe the combined type was giving a problem and I wasn't certain if it'd have been fine to work with DataProperty, but if it works, I can go for that. I think the primary issue with the code migration was that trying to convert it directly to the combined type is that, at least to the TS version we're using, it identified it as not being able to extend a type.

Worst case I can just throw an ignore and a dev note on the non optionals.

This should at least be enough to get it to compile.

Prinsn commented 3 years ago

I've been able to complete the migration with no errors shown as of yet.

I'll leave this open for any type of tracking of what might want to be moved into documentation, but I think I'm good. 👍

steveschmitt commented 3 years ago

That's good news. The UPGRADE doc is pretty sparse. Can you summarize the things that we need to add to the documentation?

Prinsn commented 3 years ago

I can try, but this is like a month of split effort.

Reviewing this document, I think the only not obvious things are migrating from breeze-client-labs...

I can't recall anything specific other than confusion, as I was able to whittle it down over time. The issues with this.masterManager.exportEntities() expecting arguments seems to have been more an erroneous issue than a problem with the client, so it might not have any glaring issues?

steveschmitt commented 3 years ago

Ok, thanks. If you think of anything else, please write it here.

Prinsn commented 2 years ago

Updating since I ran into this just now:

the step

If you have this:

import 'breeze-client/breeze.dataService.webApi';
import 'breeze-client/breeze.modelLibrary.backingStore';
import 'breeze-client/breeze.uriBuilder.odata';
import { BreezeBridgeHttpClientModule } from 'breeze-bridge2-angular';
Replace it with this:

import { DataServiceWebApiAdapter } from 'breeze-client/adapter-data-service-webapi';
import { ModelLibraryBackingStoreAdapter } from 'breeze-client/adapter-model-library-backing-store';
import { UriBuilderODataAdapter } from 'breeze-client/adapter-uri-builder-odata';
import { AjaxHttpClientAdapter } from 'breeze-client/adapter-ajax-httpclient';
Note that now you do not need breeze-bridge2-angular, because the AjaxHttpClientAdapter is now part of the breeze-client package.

Then, in your constructor function (for your module or Entity Manager Provider):

constructor(http: HttpClient) {
    // the order is important
    ModelLibraryBackingStoreAdapter.register();
    UriBuilderODataAdapter.register();
    AjaxHttpClientAdapter.register(http);
    DataServiceWebApiAdapter.register();
}

Seems to have at least one element of it as being required, we did not use any but one of these, so "replacing the imports" made it sound like "if you use these, then make sure to add this to your contsructor"