Convert to TypeScript?

[brodycj]

Someone asked me by email if there would be any objections to converting this project to TypeScript in a pull request.

This kind of a contribution is always welcome for discussion and consideration. It would be best to first raise an issue for discussion.

My personal feelings about this idea are coming in a comment. I will also cast a vote on my own issue, based on my personal feelings coming up.

[breautek]

Pros:

• Automated typings generated which can be included in package.json which is useful for typescript projects that uses the library
• Well defined interfaces in my opinion can make code more understandable.
• Compile time checks can help avoid stupid mistakes that otherwise will only be caught if you happen to trigger the execution during runtime.
• Using editors with some kind of typescript intellisense such as VS Code really makes working with the codebase easier, even if you're unfamiliar with the API because exploring the API is right there in your editor as you type.
• Generally speaking, enforces good coding practices (by enforcing you to use consistent types, and not mixing strings with numbers, etc)

Cons:

• Typescript can make the code feel cumbersome with all the type declarations. (Personally I see this as self-documenting, but I know a lot of people that do feel this way...)
• In my experience, developers who primarily work in JavaScript and nothing else sometimes have trouble getting into a strict-type paradigm, especially if they never used a strict-typed language before.
• Extending the above point, it may discourage contributors, but this is just speculation.

Personally I love Typescript, but whether I support it here or not, I don't know yet.

[brodycj]

I would actually downvote this idea (yes my own proposal!) for the following reasons:

• Simpler idea would be to use JSDoc comments for strong static typing, as I proposed in Use JSDoc with tsc for strong static typing #65
• I found https://github.com/NativeScript/pbxproj-dom which seems to already provide similar functionality implemented in TypeScript
• TypeScript has additional complexity in the language which provides little if any benefit to the end user

I think there are some excellent points by @ericelliott in the following articles:

• https://medium.com/javascript-scene/the-typescript-tax-132ff4cb175b
• https://medium.com/javascript-scene/you-might-not-need-typescript-or-static-types-aa7cb670a77b
• https://medium.com/javascript-scene/the-shocking-secret-about-static-types-514d39bf30a3
• https://medium.com/javascript-scene/angular-2-vs-react-the-ultimate-dance-off-60e7dfbc379c

Thanks @breautek for the quick reaction!

[breautek]

TypeScript has additional complexity in the language which provides little if any benefit to the end user

This was the primarily reason why I wasn't like "YES LETS DO IT!"

[cordiosd]

I was the person who contacted @brodybits about converting this to typescript. It appears as if our organization may be requiring a significant number of Cordova apps and modifying the IOS projects in plugins has been a significant issue. I had started creating a .d.ts file for this library only to estimate that doing this correctly was on the order of magnitude similar to converting the code to typescript.

So I bit the bullet and did it. I know the community might not appreciate typescript and might not pull this into the current repository. And if so, that is fine. If that is the case, I will probably just maintain this typescript version of the library for our internal uses and remove some of the inconsistencies that were required to allow the current tests to continue to pass and support the current API for code that might have been deployed against this package.

If you are interested in seeing what this looks like and hopefully providing some feedback, the code is sitting in the github branch:

https://github.com/dball-adashi/cordova-node-xcode/tree/typescript-migration-eval

At the present time I have:

1. All source except for the generated pegjs parsing code is maintained in typescript.
2. the npm build script regenerates the .js and the associated .d.ts files and reassembles the javascript files where they were before so current clients do not break.
3. There is now an environment variable that allows changing the parse logic to not force using a fork. I had problems debugging failed tests with the previous logic.
4. I have applied some DRY principles to the internals where the same code had been copied and pasted to facilitate similar scenarios.
5. I borrowed from the only other place I could find that had addressed Xcode project files in any more depth than this repo did. That was:
   https://github.com/Monobjc/monobjc-tools/tree/master/Monobjc.Tools/Xcode
6. The development workflow works pretty good for Visual Studio Code and I have committed the vs code configuration files to the repo.

At the present time, I have not:

1. Tested this new version against a real xcode project
2. Modified the library to perform the sort of changes we require to xcode projects.

I.e. this is not ready for a pull.

I am new to contributing to open source and this is the first significant bit of code that I have ported to Typescript. I usually write from the ground up. So I am happy and hopeful to hear any feedback about this library, version, or anything related to this effort so that I may improve my future efforts.

Thank you.

[breautek]

I can't foresee it being merged into the main repo, but I don't think there is anything wrong maintaining your own version of the library.

[brodycj]

Hi @dball-adashi, that is the beauty of open-source!

My one major criticism is that you seemed to have "borrowed" from a GPL licensed project. I think it should be obvious that this would be unacceptable in a PR on an Apache project and bad for permissive licensing in general.

In case you do need something from a GPL licensed project, I would recommend you consider moving out into its own component.

Otherwise, please feel free to update and publish as you feel would be appropriate.

AFAIC link to your fork would be welcome if you can keep the GPL code out.

[cordiosd]

Brody,
Thank you for the note on licensing. I did not realize the licensing was different. The "borrowing" was effectively retyping in the list of ISA types that appear in an xcode project file and reviewing the members of a specific PBXObject interface that the monoobj tools was supporting. Does this constitute a licensing issue?

Should I assume at this point that once I complete my version either?
A) It will not be pulled back into apache/cordova-node-xcode
B) It may be pulled back into apache/cordova-node-xcode

The reason I ask is that I removed errors and validation checks I would normally leave in a library to make sure it is being used correctly simply so that it could pass the existing unit tests. I also left improperly (inconsistently) named methods so that it would remain compatible with existing clients.
If I know I am starting without a need to support existing clients and existing tests that are redundant to the services the compiler already provides, I would make several changes to this code base that would make it better suited for only my purposes.

@breautek and @brodybits, thank you for your prompt replies and attention to this. I appreciate every comment and set of information you provide.

[brodycj]

The "borrowing" was effectively retyping in the list of ISA types that appear in an xcode project file and reviewing the members of a specific PBXObject interface that the monoobj tools was supporting. Does this constitute a licensing issue?

I would say "no", your code is OK. I think it would be good to give credit for the information that you used from the project, and how you just used the information rather than directly copying the code due to this licensing issue. And I think the fact that the other project is in a different language makes this especially clear.

Should I assume [...]

My response coming soon...

[brodycj]

Should I assume [...]

A) It will not be pulled back into apache/cordova-node-xcode

I would not make that assumption 100%. I wold also be reluctant to confirm your other assumption that it "may be pulled back into apache/cordova-node-xcode" at this point.

IMHO a fork that only does the port to TypeScript would not help a lot of other people. I would personally favor using JSDoc with tsc (TypeScript compiler) to enforce strong typing instead, as I proposed in #65.

But: if you or anyone else does make a fork in TypeScript that includes some other improvements, and with better support, it may be able to help some other users in the community. For example, I noticed that React Native does use this package for its CLI, which seems to be the one major component in TypeScript: https://github.com/react-native-community/cli/tree/master/packages/platform-ios/src

[fbartho]

In our application, I've had to add a hacky TypeScript layer (not a proper typings file) to make using the methods I needed from this library a bit easier.

I'd be thrilled to have a proper TypeScript layer. I think TypeScript is awesome for the somewhat self documenting nature of it, when I needed to add some extra features or work around some bugs it was brutally difficult to figure out what attributes were expected/allowed/where I needed to patch things just with the JS/pbxproj output.

[cordiosd]

@fbartho I would love to get your feedback.

If you replace your Xcode dependency in the package.json file with the following temporarily and disable your manually created .d.ts files from being considered by typescript, you should get a sense of the design. Your code should run unchanged. That is unless the type checking indicates there is a problem. It would still run as the logic is unchanged.

"dependencies": {
"xcode": "git+https://github.com/dball-adashi/cordova-node-xcode.git#typescript-migration-eval",
...

[murat-mehmet]

For anyone in need of this, this is the best solution so far:

npm install xcode
npx dts-gen -m xcode

This will generate a xcode.d.ts file in the project root, move it into the src folder.

1. Open xcode.d.ts, rename export class project to export class XcodeProject
2. Add export function project(filePath: string): XcodeProject;
3. Wrap everything into declare module 'xcode'

The final file should look like this:

declare module 'xcode' {
  export function project(filePath: string): XcodeProjectType;
  export class XcodeProjectType {
    constructor(filename: any);

    addBuildPhase(
      filePathsArray: any,
      buildPhaseType: any,
      comment: any,
      target: any,
      optionsOrFolderType?: any,
      subfolderPath?: any
    ): any;
...

Everything is declared as any but at least function and parameter names are generated and you can modify types for the functions you need easily.

Maybe a hero sets types for all methods and shares with us.