fix #393 - excludeNotExported with default exports in two statements

[nicknisi]

Fix issue where default exports are separated across two statements such
as

const foo = {};
export default foo;

In this above example, the could would originally not be able to find foo in its reflections list because the declaration factory would return null for foo because when that statement is hit, it's not set to be exported when excludeNotExported is true. This PR fixes that by not returning null in the declaration factory, and then by pruning the list of reflections and children from the project structure later on in the converter's resolve.

• remove the short-circuit in the declaration factory so all reflections
  exist after converter#convert is run
• in converter#resolve, add check for excludeNotExported and then prune
  the references list and children in the project structure whose
  isExported flag is false

The tests do not currently run with the excludeNotExported flag enabled.

This fix is associated with some project fixes and refactoring detailed in #378 and fixes #393.

[blakeembrey]

Any chance you could quickly show what it looks like with these changes? It'll also need the PR to be updated after the last commit, sorry! Thanks for helping us out 😄

[nicknisi]

@blakeembrey I rebased the latest master into this commit.

Say I have a project that contains just this single file (test.ts):

const foo = 'foo';
export default foo;

export function bar (num: number): boolean {
	return true;
}

function baz() {
}

If I run TypeDoc with --excludeNotExported, I will get this:

Notice that the output has no mention of baz (which is correct) but it also has no mention of bar because the actual export declaration is in a separate statement on the next line. This occurs because when TD is creating the declarations in src/lib/converter/factories/declaration.ts, this method short circuits if the statement is not exported, and the declaration is not added to project.reflections. So, when src/lib/converter/nodes/export.ts goes to look for this reflection, it doesn't exist and none of its potential children do either, so it still can't be exported.

This fix changes that by preventing createDeclaration from returning null and then after the fact recursively checks the reflections on the project and removes them if their isExported flag is false. I am worried that on a larger project this may slow things down since it needs to check every reflection and potentially remove it (again, only if excludeNotExported is set), but I can't really think of a better way as every other way I tried where we short circuited the Reflection creation did not seem to work out.

So, with this change, the output will look like the following where foo and bar both appear in the output.

[blakeembrey]

Thanks. I can see the improvement. However, I think it's a little confusing. If I was looking at it, I would expect to be able to import { foo } - there's nothing there that indicates a default export. Do you think it's possible we can find a way to make this more approachable. For instance, is there a flag we can add for "default" exports. How is export = handled? Can we mirror that?

[nicknisi]

Yeah, I agree it'd be great to indicate that something is the default export, which doesn't happen currently. This fix (and master) do not handle the export = case, either. I'll look into that more as well. Thanks for the feedback!

[benmccann]

I think that handling default exports is orthogonal to the issue being discussed here and should not be a blocker.

@nicknisi any chance you'd reopen this PR?