Skip to content

Fixed some minor bugs in HybridHelper #1

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bertyhell
Copy link

  • Fix semicolons in interface
  • Import angular
  • Unify name: componentSelector in the method: downgradeComponent in class and interface
  • Minor layout improvements

also: unify name: componentSelector in the method: downgradeComponent in class and interface
options = options || {};
const inputs = options.inputs || [];
const outputs = options.outputs || [];
const component = componentClass;

angular.module(moduleName).directive(componentName, downgradeComponent({
component, inputs, outputs
angular.module(moduleName).directive(componentSelector, downgradeComponent({
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the componentSelector. Selector has a dash.
I prefer to be consistence with the AngularJS code.
Please revert this change and I will merge this pull request.

The definition of the directive function in the AngularJS code:
/** * @ngdoc method * @name angular.Module#directive * @module ng * @param {string|Object} name Directive name, or an object map of directives where the * keys are the names and the values are the factories. * @param {Function} directiveFactory Factory function for creating new instance of * directives. * @description * See {@link ng.$compileProvider#directive $compileProvider.directive()}. */ directive: invokeLaterAndSetModuleName('$compileProvider', 'directive'),

@SamanthaAdrichem
Copy link

I'm to lazy to make a pull request, but there are a few bugs in here

First of all, the import of angular is missing
Second an = is missng
And third it says: downQradeProvider instead of downgradeProvider.

Here is the full code on which my editor and webpack no longer fails.

import { downgradeComponent, downgradeInjectable } from '@angular/upgrade/static';
import { FactoryProvider } from '@angular/core';
import * as angular from 'angular';

export interface IComponentUpgradeOptions {
	inputs?: string[],
	outputs?: string[]
}

export interface IHybridHelper {
	downgradeComponent(moduleName: string, componentSelector: string, componentClass: any, options?: IComponentUpgradeOptions): IHybridHelper,
	downgradeProvider(moduleName: string, providerName: string, providerClass: any): IHybridHelper,
	buildProviderForUpgrade(ng1Name: string, ng2Name?: string): FactoryProvider
}

export const HybridHelper: IHybridHelper = {

	downgradeComponent: (moduleName: string, componentName: string, componentClass: any, options?: IComponentUpgradeOptions): IHybridHelper => {
		options = options || {};
		const inputs = options.inputs || [];
		const outputs = options.outputs || [];
		const component = componentClass;

		angular.module(moduleName).directive(componentName, downgradeComponent({
			component, inputs, outputs
		}) as angular.IDirectiveFactory);

		return HybridHelper;
	},

	downgradeProvider: (moduleName: string, providerName: string, providerClass: any): IHybridHelper => {
		angular.module(moduleName).factory(providerName, downgradeInjectable(providerClass));

		return HybridHelper;
	},

	buildProviderForUpgrade: (ng1Name: string, ng2Name?: string): FactoryProvider => {
		ng2Name = ng2Name || ng1Name;

		return {
			provide: ng2Name,
			useFactory: buildFactoryForUpgradeProvider(ng1Name),
			deps: ['$injector']
		};
	}
};

function buildFactoryForUpgradeProvider(ng1Name: string): Function {
	return (injector: any) => injector.get(ng1Name);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants