Merge pull request #2832 from NoelDeMartin/MOBILE-3320

MOBILE-3320: Navigation improvements + E2E move to Travis

.github/workflows/e2e.yml

@@ -1,45 +0,0 @@
-name: E2E
-
-on:
-  schedule:
-    - cron: '0 0 * * *'  # every day at midnight
-
-jobs:
-  e2e:
-    runs-on: ubuntu-latest
-    env:
-      MOODLE_DOCKER_DB: pgsql
-      MOODLE_DOCKER_BROWSER: chrome
-      MOODLE_DOCKER_PHP_VERSION: 7.3
-    steps:
-    - uses: actions/checkout@v2
-    - name: Use Node.js
-      uses: actions/setup-node@v1
-      with:
-        node-version: '12.x'
-    - name: Install npm packages
-      run: npm ci
-    - name: Additional checkouts
-      run: |
-        git clone --branch master --depth 1 git://github.com/moodle/moodle $GITHUB_WORKSPACE/moodle
-        git clone --branch ionic5 --depth 1 git://github.com/moodlehq/moodle-local_moodlemobileapp $GITHUB_WORKSPACE/moodle/local/moodlemobileapp
-        git clone --branch MOBILE-3738 --depth 1 git://github.com/NoelDeMartin/moodle-docker $GITHUB_WORKSPACE/moodle-docker
-    - name: Setup docker machine
-      run: |
-        export MOODLE_DOCKER_WWWROOT=$GITHUB_WORKSPACE/moodle
-        export MOODLE_DOCKER_APP_PATH=$GITHUB_WORKSPACE
-        cp $GITHUB_WORKSPACE/moodle-docker/config.docker-template.php $GITHUB_WORKSPACE/moodle/config.php
-        $GITHUB_WORKSPACE/moodle-docker/bin/moodle-docker-compose pull
-        $GITHUB_WORKSPACE/moodle-docker/bin/moodle-docker-compose up -d
-        $GITHUB_WORKSPACE/moodle-docker/bin/moodle-docker-wait-for-db
-        $GITHUB_WORKSPACE/moodle-docker/bin/moodle-docker-wait-for-app
-    - name: Init behat
-      run: |
-        export MOODLE_DOCKER_WWWROOT=$GITHUB_WORKSPACE/moodle
-        export MOODLE_DOCKER_APP_PATH=$GITHUB_WORKSPACE
-        $GITHUB_WORKSPACE/moodle-docker/bin/moodle-docker-compose exec -T webserver sh -c "php admin/tool/behat/cli/init.php"
-    - name: Run tests
-      run: |
-        export MOODLE_DOCKER_WWWROOT=$GITHUB_WORKSPACE/moodle
-        export MOODLE_DOCKER_APP_PATH=$GITHUB_WORKSPACE
-        $GITHUB_WORKSPACE/moodle-docker/bin/moodle-docker-compose exec -T webserver sh -c "php admin/tool/behat/cli/run.php --tags="@app" --auto-rerun"

.travis.yml

@@ -56,3 +56,21 @@ jobs:
     if: env(DEPLOY) = 1 AND env(BUILD_IOS) = 1
     os: osx
     osx_image: xcode12.4
+  - stage: test
+    name: "End to end tests (mod_forum and mod_messages)"
+    services:
+    - docker
+    if: type = cron
+    script: scripts/test_e2e.sh "@app&&@mod_forum" "@app&&@mod_messages"
+  - stage: test
+    name: "End to end tests (mod_course, core_course and mod_courses)"
+    services:
+    - docker
+    if: type = cron
+    script: scripts/test_e2e.sh "@app&&@mod_course" "@app&&@core_course" "@app&&@mod_courses"
+  - stage: test
+    name: "End to end tests (others)"
+    services:
+    - docker
+    if: type = cron
+    script: scripts/test_e2e.sh "@app&&~@mod_forum&&~@mod_messages&&~@mod_course&&~@core_course&&~@mod_courses"

scripts/test_e2e.sh

@@ -0,0 +1,50 @@
+#!/bin/bash
+
+source "scripts/functions.sh"
+
+# Prepare variables
+basedir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/../" && pwd )"
+dockerscripts="$HOME/moodle-docker/bin/"
+dockercompose="$dockerscripts/moodle-docker-compose"
+
+export MOODLE_DOCKER_DB=pgsql
+export MOODLE_DOCKER_BROWSER=chrome
+export MOODLE_DOCKER_WWWROOT="$HOME/moodle"
+export MOODLE_DOCKER_PHP_VERSION=7.4
+export MOODLE_DOCKER_APP_PATH=$basedir
+
+# Prepare dependencies
+print_title "Preparing dependencies"
+git clone --branch master --depth 1 git://github.com/moodle/moodle $HOME/moodle
+git clone --branch ionic5 --depth 1 git://github.com/moodlehq/moodle-local_moodlemobileapp $HOME/moodle/local/moodlemobileapp
+
+# TODO replace for moodlehq/moodle-docker after merging https://github.com/moodlehq/moodle-docker/pull/156
+git clone --branch MOBILE-3738 --depth 1 git://github.com/NoelDeMartin/moodle-docker $HOME/moodle-docker
+
+cp $HOME/moodle-docker/config.docker-template.php $HOME/moodle/config.php
+
+# Build app
+print_title "Building app"
+npm ci
+
+# Start containers
+print_title "Starting containers"
+$dockercompose pull
+$dockercompose up -d
+$dockerscripts/moodle-docker-wait-for-db
+$dockerscripts/moodle-docker-wait-for-app
+
+$dockercompose exec -T webserver sh -c "php admin/tool/behat/cli/init.php"
+notify_on_error_exit "e2e failed initializing behat"
+
+print_title "Running e2e tests"
+
+# Run tests
+for tags in "$@"
+do
+    $dockercompose exec -T webserver sh -c "php admin/tool/behat/cli/run.php --tags=\"$tags\" --auto-rerun"
+    notify_on_error_exit "Some e2e tests are failing, please review"
+done
+
+# Clean up
+$dockercompose down

src/app/app-routing.module.ts

@@ -97,6 +97,33 @@ function buildConditionalUrlMatcher(pathOrMatcher: string | UrlMatcher, conditio
     };
 }
 
+export function buildRegExpUrlMatcher(regexp: RegExp): UrlMatcher {
+    return (segments: UrlSegment[]): UrlMatchResult | null => {
+        // Ignore empty paths.
+        if (segments.length === 0) {
+            return null;
+        }
+
+        const path = segments.map(segment => segment.path).join('/');
+        const match = regexp.exec(path)?.[0];
+
+        // Ignore paths that don't match the start of the url.
+        if (!match || !path.startsWith(match)) {
+            return null;
+        }
+
+        // Consume segments that match.
+        const [consumed] = segments.slice(1).reduce(([consumed, path], segment) => path === match
+            ? [consumed, path]
+            :[
+                consumed.concat(segment),
+                `${path}/${segment.path}`,
+            ], [[segments[0]] as UrlSegment[], segments[0].path]);
+
+        return { consumed };
+    };
+}
+
 export type ModuleRoutes = { children: Routes; siblings: Routes };
 export type ModuleRoutesConfig = Routes | Partial<ModuleRoutes>;
 

src/app/tests/app-routing.module.test.ts

@@ -0,0 +1,43 @@
+// (C) Copyright 2015 Moodle Pty Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { Route } from '@angular/compiler/src/core';
+import { UrlSegment, UrlSegmentGroup } from '@angular/router';
+
+import { mock } from '@/testing/utils';
+
+import { buildRegExpUrlMatcher } from '../app-routing.module';
+
+describe('Routing utils', () => {
+
+    it('matches paths using a RegExp', () => {
+        const matcher = buildRegExpUrlMatcher(/foo(\/bar)*/);
+        const route = mock<Route>();
+        const segmentGroup = mock<UrlSegmentGroup>();
+        const toUrlSegment = (path: string) => new UrlSegment(path, {});
+        const testMatcher = (path: string, consumedParts: string[] | null) =>
+            expect(matcher(path.split('/').map(toUrlSegment), segmentGroup, route))
+                .toEqual(
+                    consumedParts
+                        ? { consumed: consumedParts.map(toUrlSegment) }
+                        : null,
+                );
+
+        testMatcher('baz/foo/bar', null);
+        testMatcher('foo', ['foo']);
+        testMatcher('foo/baz', ['foo']);
+        testMatcher('foo/bar/bar/baz', ['foo', 'bar', 'bar']);
+    });
+
+});

src/core/components/context-menu/context-menu.ts

@@ -40,13 +40,10 @@ export class CoreContextMenuComponent implements OnInit, OnDestroy {
     protected items: CoreContextMenuItemComponent[] = [];
     protected itemsMovedToParent: CoreContextMenuItemComponent[] = [];
     protected itemsChangedStream: Subject<void>; // Stream to update the hideMenu boolean when items change.
-    protected instanceId: string;
     protected parentContextMenu?: CoreContextMenuComponent;
     protected expanded = false;
 
-    constructor(
+    constructor(elementRef: ElementRef) {
-        elementRef: ElementRef,
-    ) {
         // Create the stream and subscribe to it. We ignore successive changes during 250ms.
         this.itemsChangedStream = new Subject<void>();
         this.itemsChangedStream.pipe(auditTime(250));
@@ -61,7 +58,7 @@ export class CoreContextMenuComponent implements OnInit, OnDestroy {
         // Calculate the unique ID.
         this.uniqueId = 'core-context-menu-' + CoreUtils.getUniqueId('CoreContextMenuComponent');
 
-        this.instanceId = CoreDomUtils.storeInstanceByElement(elementRef.nativeElement, this);
+        CoreDomUtils.storeInstanceByElement(elementRef.nativeElement, this);
     }
 
     /**
@@ -202,7 +199,6 @@ export class CoreContextMenuComponent implements OnInit, OnDestroy {
      * Component destroyed.
      */
     ngOnDestroy(): void {
-        CoreDomUtils.removeInstanceById(this.instanceId);
         this.removeMergedItems();
     }
 

src/core/components/navbar-buttons/navbar-buttons.ts

@@ -62,13 +62,13 @@ export class CoreNavBarButtonsComponent implements OnInit, OnDestroy {
     protected forceHidden = false;
     protected logger: CoreLogger;
     protected movedChildren?: Node[];
-    protected instanceId: string;
     protected mergedContextMenu?: CoreContextMenuComponent;
 
     constructor(element: ElementRef) {
         this.element = element.nativeElement;
         this.logger = CoreLogger.getInstance('CoreNavBarButtonsComponent');
-        this.instanceId = CoreDomUtils.storeInstanceByElement(this.element, this);
+
+        CoreDomUtils.storeInstanceByElement(this.element, this);
     }
 
     /**
@@ -138,9 +138,8 @@ export class CoreNavBarButtonsComponent implements OnInit, OnDestroy {
         }
 
         // Both containers have a context menu. Merge them to prevent having 2 menus at the same time.
-        const mainContextMenuInstance: CoreContextMenuComponent = CoreDomUtils.getInstanceByElement(mainContextMenu);
+        const mainContextMenuInstance = CoreDomUtils.getInstanceByElement<CoreContextMenuComponent>(mainContextMenu);
-        const secondaryContextMenuInstance: CoreContextMenuComponent =
+        const secondaryContextMenuInstance = CoreDomUtils.getInstanceByElement<CoreContextMenuComponent>(secondaryContextMenu);
-            CoreDomUtils.getInstanceByElement(secondaryContextMenu);
 
         // Check that both context menus belong to the same core-tab. We shouldn't merge menus from different tabs.
         if (mainContextMenuInstance && secondaryContextMenuInstance) {
@@ -247,8 +246,6 @@ export class CoreNavBarButtonsComponent implements OnInit, OnDestroy {
      * Component destroyed.
      */
     ngOnDestroy(): void {
-        CoreDomUtils.removeInstanceById(this.instanceId);
-
         // This component was destroyed, remove all the buttons that were moved.
         // The buttons can be moved outside of the current page, that's why we need to manually destroy them.
         // There's no need to destroy context menu items that were merged because they weren't moved from their DOM position.

src/core/components/tabs-outlet/tabs-outlet.ts

@@ -23,7 +23,7 @@ import {
     ElementRef,
     SimpleChange,
 } from '@angular/core';
-import { IonTabs } from '@ionic/angular';
+import { IonTabs, ViewDidEnter, ViewDidLeave } from '@ionic/angular';
 import { Subscription } from 'rxjs';
 
 import { CoreUtils } from '@services/utils/utils';
@@ -65,10 +65,11 @@ export class CoreTabsOutletComponent extends CoreTabsBaseComponent<CoreTabsOutle
     @ViewChild(IonTabs) protected ionTabs?: IonTabs;
 
     protected stackEventsSubscription?: Subscription;
+    protected outletActivatedSubscription?: Subscription;
+    protected lastActiveComponent?: Partial<ViewDidLeave>;
+    protected existsInNavigationStack = false;
 
-    constructor(
+    constructor(element: ElementRef) {
-        element: ElementRef,
-    ) {
         super(element);
     }
 
@@ -110,6 +111,9 @@ export class CoreTabsOutletComponent extends CoreTabsBaseComponent<CoreTabsOutle
                 this.showHideTabs(scrollElement.scrollTop, scrollElement);
             }
         });
+        this.outletActivatedSubscription = this.ionTabs?.outlet.activateEvents.subscribe(() => {
+            this.lastActiveComponent = this.ionTabs?.outlet.component;
+        });
     }
 
     /**
@@ -127,6 +131,35 @@ export class CoreTabsOutletComponent extends CoreTabsBaseComponent<CoreTabsOutle
         super.ngOnChanges(changes);
     }
 
+    /**
+     * @inheritdoc
+     */
+    ionViewDidEnter(): void {
+        super.ionViewDidEnter();
+
+        // The `ionViewDidEnter` method is not called on nested outlets unless the parent page is leaving the navigation stack,
+        // that's why we need to call it manually if the page that is entering already existed in the stack (meaning that it is
+        // entering in response to a back navigation from the page on top).
+        if (this.existsInNavigationStack && this.ionTabs?.outlet.isActivated) {
+            (this.ionTabs?.outlet.component as Partial<ViewDidEnter>).ionViewDidEnter?.();
+        }
+
+        // After the view has entered for the first time, we can assume that it'll always be in the navigation stack
+        // until it's destroyed.
+        this.existsInNavigationStack = true;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    ionViewDidLeave(): void {
+        super.ionViewDidLeave();
+
+        // The `ionViewDidLeave` method is not called on nested outlets unless the active view changes, that's why
+        // we need to call it manually if the page is leaving and the last active component was not notified.
+        this.lastActiveComponent?.ionViewDidLeave?.();
+    }
+
     /**
      * Load the tab.
      *
@@ -150,7 +183,7 @@ export class CoreTabsOutletComponent extends CoreTabsBaseComponent<CoreTabsOutle
         const elements = this.ionTabs!.outlet.nativeEl.querySelectorAll('core-navbar-buttons');
         const domUtils = CoreDomUtils.instance;
         elements.forEach((element) => {
-            const instance: CoreNavBarButtonsComponent = domUtils.getInstanceByElement(element);
+            const instance = domUtils.getInstanceByElement<CoreNavBarButtonsComponent>(element);
 
             if (instance) {
                 const pagetagName = element.closest('.ion-page')?.tagName;
@@ -165,6 +198,8 @@ export class CoreTabsOutletComponent extends CoreTabsBaseComponent<CoreTabsOutle
     ngOnDestroy(): void {
         super.ngOnDestroy();
         this.stackEventsSubscription?.unsubscribe();
+        this.outletActivatedSubscription?.unsubscribe();
+        this.existsInNavigationStack = false;
     }
 
 }

src/core/components/tabs/tab.ts

@@ -144,7 +144,7 @@ export class CoreTabComponent implements OnInit, OnDestroy, CoreTabBase {
     protected showHideNavBarButtons(show: boolean): void {
         const elements = this.element.querySelectorAll('core-navbar-buttons');
         elements.forEach((element) => {
-            const instance: CoreNavBarButtonsComponent = CoreDomUtils.getInstanceByElement(element);
+            const instance = CoreDomUtils.getInstanceByElement<CoreNavBarButtonsComponent>(element);
 
             if (instance) {
                 instance.forceHide(!show);

src/core/features/course/course.module.ts

@@ -40,6 +40,7 @@ import { CoreCourseOptionsDelegateService } from './services/course-options-dele
 import { CoreCourseOfflineProvider } from './services/course-offline';
 import { CoreCourseSyncProvider } from './services/sync';
 import { COURSE_INDEX_PATH } from '@features/course/course-lazy.module';
+import { buildRegExpUrlMatcher } from '@/app/app-routing.module';
 
 export const CORE_COURSE_SERVICES: Type<unknown>[] = [
     CoreCourseProvider,
@@ -59,7 +60,7 @@ export const COURSE_CONTENTS_PATH = `${COURSE_PAGE_NAME}/${COURSE_INDEX_PATH}/${
 
 const routes: Routes = [
     {
-        path: COURSE_PAGE_NAME,
+        matcher: buildRegExpUrlMatcher(new RegExp(`^${COURSE_PAGE_NAME}(/deep)*`)),
         loadChildren: () => import('@features/course/course-lazy.module').then(m => m.CoreCourseLazyModule),
     },
 ];

src/core/features/course/pages/index/index.page.ts

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 import { Component, ViewChild, OnDestroy, OnInit } from '@angular/core';
-import { Params } from '@angular/router';
+import { ActivatedRoute, Params } from '@angular/router';
 
 import { CoreTabsOutletTab, CoreTabsOutletComponent } from '@components/tabs-outlet/tabs-outlet';
 import { CoreCourseFormatDelegate } from '../../services/format-delegate';
@@ -54,7 +54,7 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy {
         pageParams: {},
     };
 
-    constructor() {
+    constructor(private route: ActivatedRoute) {
         this.selectTabObserver = CoreEvents.on(CoreEvents.SELECT_COURSE_TAB, (data) => {
             if (!data.name) {
                 // If needed, set sectionId and sectionNumber. They'll only be used if the content tabs hasn't been loaded yet.
@@ -81,6 +81,11 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy {
      * Component being initialized.
      */
     async ngOnInit(): Promise<void> {
+        // Increase route depth.
+        const path = CoreNavigator.getRouteFullPath(this.route.snapshot);
+
+        CoreNavigator.increaseRouteDepth(path.replace(/(\/deep)+/, ''));
+
         // Get params.
         this.course = CoreNavigator.getRouteParam('course');
         this.firstTabName = CoreNavigator.getRouteParam('selectedTab');
@@ -180,6 +185,9 @@ export class CoreCourseIndexPage implements OnInit, OnDestroy {
      * Page destroyed.
      */
     ngOnDestroy(): void {
+        const path = CoreNavigator.getRouteFullPath(this.route.snapshot);
+
+        CoreNavigator.decreaseRouteDepth(path.replace(/(\/deep)+/, ''));
         this.selectTabObserver?.off();
     }
 

src/core/features/course/services/handlers/default-format.ts

@@ -177,7 +177,11 @@ export class CoreCourseFormatDefaultHandler implements CoreCourseFormatHandler {
         Object.assign(params, { course: course });
 
         // Don't return the .push promise, we don't want to display a loading modal during the page transition.
-        CoreNavigator.navigateToSitePath(`course/${course.id}`, { params });
+        const currentTab = CoreNavigator.getCurrentMainMenuTab();
+        const routeDepth = CoreNavigator.getRouteDepth(`/main/${currentTab}/course/${course.id}`);
+        const deepPath = '/deep'.repeat(routeDepth);
+
+        CoreNavigator.navigateToSitePath(`course${deepPath}/${course.id}`, { params });
     }
 
     /**

src/core/services/navigator.ts

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 import { Injectable } from '@angular/core';
-import { ActivatedRoute, Params } from '@angular/router';
+import { ActivatedRoute, ActivatedRouteSnapshot, Params } from '@angular/router';
 
 import { NavigationOptions } from '@ionic/angular/providers/nav-controller';
 
@@ -71,6 +71,7 @@ export type CoreNavigatorCurrentRouteOptions = Partial<{
 @Injectable({ providedIn: 'root' })
 export class CoreNavigatorService {
 
+    protected routesDepth: Record<string, number> = {};
     protected storedParams: Record<number, unknown> = {};
     protected lastParamId = 0;
 
@@ -397,6 +398,38 @@ export class CoreNavigatorService {
         return pageComponent || routeData ? null : route;
     }
 
+    /**
+     * Increase the number of times a route is repeated on the navigation stack.
+     *
+     * @param path Absolute route path.
+     */
+    increaseRouteDepth(path: string): void {
+        this.routesDepth[path] = this.getRouteDepth(path) + 1;
+    }
+
+    /**
+     * Decrease the number of times a route is repeated on the navigation stack.
+     *
+     * @param path Absolute route path.
+     */
+    decreaseRouteDepth(path: string): void {
+        if (this.getRouteDepth(path) <= 1) {
+            delete this.routesDepth[path];
+        } else {
+            this.routesDepth[path]--;
+        }
+    }
+
+    /**
+     * Get the number of times a route is repeated on the navigation stack.
+     *
+     * @param path Absolute route path.
+     * @return Route depth.
+     */
+    getRouteDepth(path: string): number {
+        return this.routesDepth[path] ?? 0;
+    }
+
     /**
      * Navigate to a path within the main menu.
      * If the path belongs to a visible tab, that tab will be selected.
@@ -493,16 +526,16 @@ export class CoreNavigatorService {
     /**
      * Get the full path of a certain route, including parent routes paths.
      *
-     * @param route Route.
+     * @param route Route snapshot.
      * @return Path.
      */
-    getRouteFullPath(route: ActivatedRoute | null): string {
+    getRouteFullPath(route: ActivatedRouteSnapshot | null): string {
         if (!route) {
             return '';
         }
 
         const parentPath = this.getRouteFullPath(route.parent);
-        const routePath = route.snapshot.url.join('/');
+        const routePath = route.url.join('/');
 
         if (!parentPath && !routePath) {
             return '';

src/core/services/utils/dom.ts

@@ -53,9 +53,7 @@ export class CoreDomUtilsProvider {
     protected template: HTMLTemplateElement = document.createElement('template'); // A template element to convert HTML to element.
 
     protected matchesFunctionName?: string; // Name of the "matches" function to use when simulating a closest call.
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    protected instances: WeakMap<Element, unknown> = new WeakMap(); // Store component/directive instances indexed by element.
-    protected instances: {[id: string]: any} = {}; // Store component/directive instances by id.
-    protected lastInstanceId = 0;
     protected debugDisplay = false; // Whether to display debug messages. Store it in a variable to make it synchronous.
     protected displayedAlerts: Record<string, HTMLIonAlertElement> = {}; // To prevent duplicated alerts.
     protected activeLoadingModals: CoreIonLoadingElement[] = [];
@@ -706,11 +704,8 @@ export class CoreDomUtilsProvider {
      * @param element The root element of the component/directive.
      * @return The instance, undefined if not found.
      */
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    getInstanceByElement<T = unknown>(element: Element): T | undefined {
-    getInstanceByElement(element: Element): any {
+        return this.instances.get(element) as T;
-        const id = element.getAttribute(this.INSTANCE_ID_ATTR_NAME);
-
-        return id && this.instances[id];
     }
 
     /**
@@ -1629,16 +1624,9 @@ export class CoreDomUtilsProvider {
      *
      * @param element The root element of the component/directive.
      * @param instance The instance to store.
-     * @return ID to identify the instance.
      */
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    storeInstanceByElement(element: Element, instance: unknown): void {
-    storeInstanceByElement(element: Element, instance: any): string {
+        this.instances.set(element, instance);
-        const id = String(this.lastInstanceId++);
-
-        element.setAttribute(this.INSTANCE_ID_ATTR_NAME, id);
-        this.instances[id] = instance;
-
-        return id;
     }
 
     /**