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.

Sign up for GitHub

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

Jump to bottom

Fixed skipping of nested object literals with --excludeNotExported #1103

Closed
wants to merge 1 commit into from
Closed

Fixed skipping of nested object literals with --excludeNotExported #1103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/lib/converter/factories/declaration.ts
View file
Open in desktop
Expand Up @@ -56,6 +56,10 @@ export function createDeclaration(context: Context, node: ts.Declaration, kind:
let isExported: boolean;
if (container.kindOf([ReflectionKind.Module, ReflectionKind.ExternalModule])) {
isExported = false; // Don't inherit exported state in modules and namespaces
} else if (container.kindOf([ReflectionKind.TypeLiteral]) && context.converter.excludeNotExported) {
// If the container for this is a type literal, it never had the
// isExported flag set on it. See https://github.com/TypeStrong/typedoc/issues/953
isExported = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This worries me. Is there a reason we can't set the isExported flag on containers when creating them instead of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I looked into that. That does seem like the more "correct" way to go about it. However, after extensive debugging I discovered that this factory function doesn't ever get called for object literals themselves. Instead, I believe the associated Declaration is created here: https://github.com/TypeStrong/typedoc/blob/master/src/lib/converter/types/reference.ts#L127

Perhaps there is an underlying issue where object literal declarations follow a different code path. I didn't want to go there, so I just wrote some tests to determine the least invasive fix, which is what you see.

Happy to dive back in if you can provide some additional insights about code structure. It's a pretty challenging codebase. :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense, thanks. I would like to fix this the right way, but haven't had enough time to dig in and see where that would be yet, so unfortunately I can't provide any guidance here. Maybe next weekend.

Yes, the codebase is rather difficult to follow, we're working on it, but its slow going. :)

} else {
isExported = container.flags.isExported;
}
Expand Down Expand Up @@ -108,7 +112,7 @@ export function createDeclaration(context: Context, node: ts.Declaration, kind:
child.setFlag(ReflectionFlag.Static, isStatic);
child.setFlag(ReflectionFlag.Private, isPrivate);
child.setFlag(ReflectionFlag.ConstructorProperty, isConstructorProperty);
child.setFlag(ReflectionFlag.Exported, isExported);
child.setFlag(ReflectionFlag.Exported, isExported);
child = setupDeclaration(context, child, node);

if (child) {
Expand Down
16 changes: 16 additions & 0 deletions src/test/converter.test.ts
View file
Open in desktop
Expand Up @@ -156,6 +156,7 @@ describe('Converter with excludeNotExported=true', function() {
const base = Path.join(__dirname, 'converter');
const exportWithLocalDir = Path.join(base, 'export-with-local');
const classDir = Path.join(base, 'class');
const literalsDir = Path.join(base, 'export-literals');
let app: Application;

before('constructs', function() {
Expand Down Expand Up @@ -204,4 +205,19 @@ describe('Converter with excludeNotExported=true', function() {
});
});

describe('export-literals', () => {
it('converts fixtures', function() {
resetReflectionID();
result = app.convert(app.expandInputFiles([literalsDir]));
Assert(result instanceof ProjectReflection, 'No reflection returned');
});

it('matches specs', function() {
const specs = JSON.parse(FS.readFileSync(Path.join(literalsDir, 'specs-without-exported.json')).toString());
let data = JSON.stringify(result!.toObject(), null, ' ');
data = data.split(normalizePath(base)).join('%BASE%');

compareReflections(JSON.parse(data), specs);
});
});
});
20 changes: 20 additions & 0 deletions src/test/converter/export-literals/export-literals.ts
View file
Open in desktop
@@ -0,0 +1,20 @@
export interface INestedInterface
{
nestedOptional?: {
innerMember: string;
};

nested: {
isIncluded: boolean;
};
}

export function func(param: {nested: string}): boolean {
return param.nested === 'yes';
}

export function createSomething() {
return {
foo: 'bar'
};
}