Skip to content
This repository was archived by the owner on Jan 23, 2022. It is now read-only.

Conversation

baronfel
Copy link

@baronfel baronfel commented May 6, 2016

A few changes to add in support for ExtJS 6. I had to add a few packages to the jsduck listing, because Sencha has done some reorganization of the sencha source code.
In addition, they package differently depending on if you use the modern or classic toolkits, so I added a theme switcher. It may be nice to generate all configurations (classic/modern) for a particular version?

In addition, I got a few warnings when generating these files, mostly around how a few Ext classes could not be found in the sources I provided (Ext.Component, Ext.container.Container, and Ext.dom.Element were the biggest ones). I didn't find these sources anywhere in the downloaded zip. Maybe you have better luck?

generator.ts Outdated


function lookupMember(members: jsduck.Member[], name: string, tagnames?: string[], static?: boolean):jsduck.Member {
function lookupMember(members: jsduck.Member[], name: string, tagnames?: string[], isstatic?: boolean):jsduck.Member {
Copy link
Owner

Choose a reason for hiding this comment

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

Could isstatic be called isStatic instead to match the rest of the file?

@Dretch
Copy link
Owner

Dretch commented May 6, 2016

Wow, thanks for putting in the effort to add ExtJS 6 support!

I will merge it if you fix the small issues I have put comments on.

Regarding your questions, I have not used ExtJS6 so I am unfortunately not best placed to answer them intelligently. I did notice that the generated type declarations file for 6.0.1 is half the size of the one for 5.1.0, which suggests there might be stuff missing.

shelljs.exec('unzip -uq *.zip');
shelljs.mkdir('-p', version.folder + '.docs');

var srcPath = version.theme ? version.theme + '/' + version.theme + '/src' : '/src';
Copy link
Owner

Choose a reason for hiding this comment

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

This variable is not used!

@baronfel
Copy link
Author

baronfel commented May 6, 2016

@Dretch Thanks for the review! I'm not very confident that these are correct at all, so I'll make your fixes and see. I want to spend a bit more time trying to integrate this at my work before saying this is ok. I'm definitely not certain that I have all of the required Ext members.

@Dretch
Copy link
Owner

Dretch commented May 7, 2016

@baronfel

I will leave off merging this until you are happier with it, then. Thanks for addressing my comments 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants