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

Merge Options in FileEmitter is broken. #6

Closed
Logical-sh opened this issue Oct 25, 2017 · 14 comments
Closed

Merge Options in FileEmitter is broken. #6

Logical-sh opened this issue Oct 25, 2017 · 14 comments
Labels

Comments

@Logical-sh
Copy link

The merge options in the file emitter merges the class options (and likely many more) with themselves instead of the intended target. This results in the property options being lost.
It looks like this effects ALOT of the option merges.

if(options.propertyEmitOptions) {
    if(options.classEmitOptions) {
        options.classEmitOptions.propertyEmitOptions = 
	        this.optionsHelper.mergeOptions(
	            options.classEmitOptions,
		    options.classEmitOptions.propertyEmitOptions);
    }
    ....
}
@ffMathy
Copy link
Owner

ffMathy commented Oct 26, 2017

This is currently being fixed in the branch here: https://github.com/ffMathy/FluffySpoon.JavaScript.CSharpToTypeScriptGenerator/tree/typescript-compiler-service

It is a quite big upgrade to the emitter, and also changes how inheritance is done.

@ffMathy ffMathy added the bug label Oct 26, 2017
@ffMathy
Copy link
Owner

ffMathy commented Dec 26, 2017

I continued working on this. I hope to get it fixed within the new year.

@jannesiera
Copy link

What is the status on this?

@ffMathy
Copy link
Owner

ffMathy commented Mar 22, 2018

I expect to be done some time next week. See the other branch for more details.

@ffMathy
Copy link
Owner

ffMathy commented Mar 23, 2018

Merging now works in the feature branch, but tests are failing. I know what's causing it though but don't have time to work on it until Monday.

@ffMathy
Copy link
Owner

ffMathy commented Mar 28, 2018

I have been ill now for a while and have hence had a bit of time to work on it. Amount of failing tests as of the day before yesterday is now down to around 15 instead of 29.

This project is now also my top priority.

@ffMathy
Copy link
Owner

ffMathy commented Mar 28, 2018

Here is the branch with progress if you want to help out.

@ffMathy
Copy link
Owner

ffMathy commented Mar 30, 2018

One failing test remaining.

@jannesiera
Copy link

@ffMathy thanks for the great work. I don't have time to help out now, but I will test my use case the coming week.

@ffMathy
Copy link
Owner

ffMathy commented Mar 31, 2018

If I make it. The last test requires some refactoring, but I am getting there.

When this issue is resolved I can continue: microsoft/TypeScript#23042

@ffMathy
Copy link
Owner

ffMathy commented Apr 1, 2018

It has now been merged. I will update the README.md file to reflect the changes, because the way defaults are set globally has now changed slightly.

@ffMathy ffMathy closed this as completed Apr 1, 2018
@ffMathy
Copy link
Owner

ffMathy commented Apr 1, 2018

I just upgraded the documentation heavily as well. Enjoy! The library is also now 100% based on the TypeScript compiler API under the hood, and allows working directly with TypeScript objects if you wish.

@ffMathy
Copy link
Owner

ffMathy commented Apr 4, 2018

Oh, and the package has now been put on NPM as well. That was about time.

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

No branches or pull requests

3 participants