elements icon indicating copy to clipboard operation
elements copied to clipboard

Multiple Configs are lost

Open adickson311 opened this issue 6 years ago • 6 comments

If you make multiple calls to LazyElementsModule.forFeature(options), ... you will only get the last configuration and lose references to other configs.

angular-extension bug

In LazyElementModule constructor:

    if (elementConfigsMultiProvider && elementConfigsMultiProvider.length) {
      const lastAddedConfigs =
        elementConfigsMultiProvider[elementConfigsMultiProvider.length - 1];
      lazyElementsLoaderService.addConfigs(lastAddedConfigs);
    }

This should be:

const configs = [].concat.apply([], elementConfigsMultiProvider);
lazyElementsLoaderService.addConfigs(configs);

adickson311 avatar Sep 20 '19 14:09 adickson311

Submitted a PR: https://github.com/angular-extensions/elements/pull/18

adickson311 avatar Sep 20 '19 14:09 adickson311

@adickson311 but is it ? the configs are registered in the service right ? so service holds them all, and whenever we call forFeature then we basically append the last one in the service or ?

 addConfigs(newConfigs: ElementConfig[]) {
    newConfigs.forEach(newConfig => {
      const existingConfig = this.getElementConfig(newConfig.tag);
      if (existingConfig) {
        console.warn(
          `${LOG_PREFIX} - ElementConfig for tag '${newConfig.tag}' was previously added, it will not be added multiple times, continue...`
        );
      } else {
        this.configs.push(newConfig);    // <--- this line 
      }
    });
  }

tomastrajan avatar Sep 20 '19 19:09 tomastrajan

@adickson311 because the constructors should happen one by one and NOT concurrently, that way we should also append configs to the service one by one and not lose anything ?

tomastrajan avatar Sep 20 '19 19:09 tomastrajan

@adickson311 the only way I could reproduce your bug is

@NgModule({
  schemas: [CUSTOM_ELEMENTS_SCHEMA],
  imports: [
    LazyElementsModule.forFeature(options1),
    LazyElementsModule.forFeature(options2),
  ]
})
export class ExampleModule {}

Where only the second option2 will be added, then again, does such an code make sense? It works great with single LazyElementsModule.forFeature(allFeatureOptions) registration...

The usage of last config prevents us from adding single config multiple times because then every new feature would re-add all the previous configs (because of the multi provider) and spam user with warning like this...

@angular-extensions/elements - ElementConfig for tag 'ion-button' was previously added, it will not be added multiple times, continue...

tomastrajan avatar Sep 27 '19 07:09 tomastrajan

@adickson311 do you have any other way to reproduce this behavior ?

tomastrajan avatar Sep 27 '19 07:09 tomastrajan

@adickson311 is this still an issue?

felipeplets avatar Sep 29 '20 02:09 felipeplets