Skip to content
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

Define protocol for porting abstract classes from .NET to TypeScript #2616

Closed
stevengum opened this issue Jul 30, 2020 · 1 comment
Closed
Assignees
Labels
Area: Engineering Internal issues that are related to improving code quality, refactorings, code cleanup, etc. P0 Must Fix. Release-blocker Size: M The issue is not very complex and it is well understood, it will take 1 to 3 days to complete team-agility An issue targeted to reduce friction to the SDK's development process.

Comments

@stevengum
Copy link
Member

stevengum commented Jul 30, 2020

IMHO - it is fine to have an abstract base class for providing default implementation shared across derived classes. However, I don't think this has to mean removing the interfaces. By removing the interface, the other classes become dependent on a specific implementation. Interfaces do a good job of defining the contract in Typescript and get compiled away. There aren't the same tradeoffs between ABCs and interfaces like there are in .NET.

Originally posted by @GeoffCoxMSFT in #2342 (comment)

I agree with Geoff's PR feedback on #2342. If there are implementations that should be default, then an abstract class is valid in TypeScript (TypeScript does not have default implementations available via interface like C#).

However, as he indicates, there are language differences that need to be taken into account when porting abstract classes into TypeScript. The point about TypeScript interfaces is especially poignant as TypeScript is a superscript layered on top of JavaScript.

While this may be more in the vein of a "'magic wand' ask" ideally, any abstract classes with provided functionality should be nigh-close to perfect, otherwise we run overload and method deprecation issues experienced in Skills in TypeScript.

We should define a protocol for porting over abstract classes and interfaces from .NET to TypeScript.

@stevengum stevengum changed the title Define pattern for porting abstract classes from .NET to TypeScript Define protocol for porting abstract classes from .NET to TypeScript Jul 30, 2020
@stevengum stevengum added BF Agility For Issues or Feature Requests that impact the SDK Team's agility Area: Engineering Internal issues that are related to improving code quality, refactorings, code cleanup, etc. P0 Must Fix. Release-blocker R11 labels Jul 30, 2020
@gabog gabog added this to the R11 milestone Sep 2, 2020
@gabog gabog removed the R11 label Sep 2, 2020
@joshgummersall
Copy link
Contributor

Deliverables would include linting and a documented guideline that lives somewhere.

@joshgummersall joshgummersall self-assigned this Sep 22, 2020
@joshgummersall joshgummersall added team-agility An issue targeted to reduce friction to the SDK's development process. Size: M The issue is not very complex and it is well understood, it will take 1 to 3 days to complete and removed BF Agility For Issues or Feature Requests that impact the SDK Team's agility labels Sep 22, 2020
@joshgummersall joshgummersall modified the milestones: R11, R12 Oct 15, 2020
@joshgummersall joshgummersall removed this from the R12 milestone Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Engineering Internal issues that are related to improving code quality, refactorings, code cleanup, etc. P0 Must Fix. Release-blocker Size: M The issue is not very complex and it is well understood, it will take 1 to 3 days to complete team-agility An issue targeted to reduce friction to the SDK's development process.
Projects
None yet
Development

No branches or pull requests

3 participants