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

Warnings caused by renaming reflections not included in the documentation #1166

Closed
1 task done
doberkofler opened this issue Jan 13, 2020 · 17 comments
Closed
1 task done
Labels
discussion Discussion on large effort changes to TypeDoc

Comments

@doberkofler
Copy link

Expected Behavior

No warnings

Actual Behavior

When running the version 0.16.2 of typedoc I get the following warnings:

Warning: Some names refer to reflections that do not exist.
This can be caused by exporting a reflection only under a different name than it is declared when excludeNotExported is set
or by a plugin removing reflections without removing references. The names that cannot be resolved are:
size
clone
defaults
assignIn
forEach
map
mapValues
find
findIndex
findKey
isEmpty
isMatch
debounce
compact
reverse
includes
without
difference
omit
union
uniq
filter
has
sortBy
head
last
template
escape
isEqual

Steps to reproduce the bug

npx typedoc --out doc/ src/ods/lodash.ts

src/ods/lodash.ts:

/**
* JavaScript utility library
*	@module lodash
*/

/* eslint-disable no-restricted-imports */
import _size from 'lodash/size';
import _clone from 'lodash/clone';
import _defaults from 'lodash/defaults';
import _assignIn from 'lodash/assignIn';
import _forEach from 'lodash/forEach';
import _map from 'lodash/map';
import _mapValues from 'lodash/mapValues';
import _find from 'lodash/find';
import _findIndex from 'lodash/findIndex';
import _findKey from 'lodash/findKey';
import _isEmpty from 'lodash/isEmpty';
import _isMatch from 'lodash/isMatch';
import _debounce from 'lodash/debounce';
import _compact from 'lodash/compact';
import _reverse from 'lodash/reverse';
import _includes from 'lodash/includes';
import _without from 'lodash/without';
import _difference from 'lodash/difference';
import _omit from 'lodash/omit';
import _union from 'lodash/union';
import _uniq from 'lodash/uniq';
import _filter from 'lodash/filter';
import _has from 'lodash/has';
import _sortBy from 'lodash/sortBy';
import _head from 'lodash/head';
import _last from 'lodash/last';
import _template from 'lodash/template';
import _escape from 'lodash/escape';
import _isEqual from 'lodash/isEqual';
/* eslint-enable no-restricted-imports */

export {
	_size as size,
	_clone as clone,
	_defaults as defaults,
	_assignIn as assignIn,
	_forEach as forEach,
	_map as map,
	_mapValues as mapValues,
	_find as find,
	_findIndex as findIndex,
	_findKey as findKey,
	_isEmpty as isEmpty,
	_isMatch as isMatch,
	_debounce as debounce,
	_compact as compact,
	_reverse as reverse,
	_includes as includes,
	_without as without,
	_difference as difference,
	_omit as omit,
	_union as union,
	_uniq as uniq,
	_filter as filter,
	_has as has,
	_sortBy as sortBy,
	_head as head,
	_last as last,
	_template as template,
	_escape as escape,
	_isEqual as isEqual,
};

Environment

  • Typedoc version: 0.16.2
  • Node.js version: 12.13.1
  • OS: Windows 8
@doberkofler doberkofler added the bug Functionality does not match expectation label Jan 13, 2020
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 13, 2020

What would you expect to happen when you export local variables under a different name?

There was a lot of consideration for how to handle this, starting under the misc notes in #801.

It sounds like this module should potentially not be included in your documentation at all. If you change the top comment to:

/**
* JavaScript utility library
*	@module lodash
* @ignore
* @packageDocumentation
*/

The module will be excluded from your documentation, removing these errors.

@Gerrit0 Gerrit0 added discussion Discussion on large effort changes to TypeDoc and removed bug Functionality does not match expectation labels Jan 13, 2020
@Gerrit0 Gerrit0 changed the title Warnings after upgrading from 0.15 to 0.16 Warnings caused by renaming reflections not included in the documentation Jan 13, 2020
@doberkofler
Copy link
Author

I scanned the notes #801 and was amazed on how much consideration was put into this PR.
I mostly opened this SR because before upgrading to typedoc 0.16 I had no warning and now there are a lot of warning in several modules that use this pattern.
Not including the modules works around the warning but just from the perspective of a simple typedoc user (and not really understanding the complexity of the problem) I would clearly prefer to have documentation for the module that "reflects" the imported symbols.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 14, 2020

By reflects do you mean copies the documentation?

@doberkofler
Copy link
Author

Yes

@doberkofler
Copy link
Author

I btw also tried to get rid of the warning by adding the following header

/**
* JavaScript utility library
*	@module utility
*	@ignore
*	@packageDocumentation
*/

but it still shows the warnings.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 14, 2020

That's weird, my best guess is that the module name plugin is messing with it... I can't reproduce locally.

Thoughts on a new flag --ignoreBrokenReferences which removes the above warning and removes references which cannot be resolved from the project?

@doberkofler
Copy link
Author

I'm not sure I really understand what is going on yet and created a simple test case:

size.ts:

export default function size(a: Array<any>): number {
	return a.length;
}

test.ts:

import _size from './size';
export {
	_size as size,
};

and run npx typedoc --out doc/ src/ods/test.ts and it workes like a charm and generates exactly the documentation that I would expect.

Could you help me understanding what is different to my original use case?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 14, 2020

The difference in your original documentation is that the eventually targeted symbol is not included in your documentation. Your generated docs don't include documentation for lodash functions.

In your reproduction, the re-exported symbol is exported.


(It's a bit more complicated than this, and it looks like we do have a bug, as you will see if you rename _size to `size, I think this is because we don't convert import statements)

@doberkofler
Copy link
Author

Still not sure I fully understand.

If I just change test.ts to:

//import _size from './size';
import _size from 'lodash/size';

export {
	_size as size,
};

I again see the warning and the only difference is where I import from.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 14, 2020

Sorry, my first attempt at responding to this was bad, edit should have clarified a bit... but still doesn't capture everything.

The problem is when TypeDoc determines if a symbol should be included in the documentation. This happens at two times.

  1. Before the reflection is created - This is when flags like --excludeNotExported, --excludeExternals , and --includeDeclarations are applied. It provides a (significant, last I measured) performance benefit to not document and then throw away documentation for files not documented.
  2. After all reflections have been created for a project - This is when @hidden/@ignore/@internal modifiers are applied to remove reflections that the user specified should not be in the project via a comment.

The first case is the problem here. With:

import _size from 'lodash/size';

export {
	_size as size,
};

TypeDoc tries to create a reference called size from test.ts to the default export of lodash/size. At this point, it doesn't know that lodash/size is outside of your generated documentation.

Then, once the whole project has been converted, we look back at all of the references and notice that there is a reference from size in test.ts to some symbol that is not included in the documentation. This is then reported as a broken link.

This might actually be fixable if we can determine at conversion time if the referenced symbol will not be included in the documentation. I'll look into it.

Thanks for the questions! Its helped me think through this a bit more thoroughly.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 14, 2020

This might actually be fixable if we can determine at conversion time if the referenced symbol will not be included in the documentation. I'll look into it.

This isn't quite ideal. Determining this and not creating a reference for external dependencies is fine, but with --excludeNotExported and the following code, I think we should still create a broken reference. Silently removing a would be confusing since parts of the documentation just disappeared, and copying _a to a isn't a good solution as discussed under "Misc Notes" in #801.

/**
 * Some documentation that refers to _a
 */
const _a = 1
export { _a as a }

Gerrit0 added a commit that referenced this issue Jan 14, 2020
@doberkofler
Copy link
Author

I wanted to add some background information on my practical use cases to eventually come up with a solution. I currently have about 100 similar warnings from modules that abstract some 3rd partly libraries and would really like to get rid of them. Typically the modules export some local bindings and re-export some imported ones.

I see the following options:

  • Skipping the module altogether does not seem to work for me but would also prevent the locally defined binding not to have any documentation.

  • Skipping the re-exported binding would solve the problem with warnings but it would actually hide the bindings in the module that are actually available.

  • It's probably far to complicated but (at least) for my uses cases the "ideal" solution would be to generate documentation for all exports including the ones that are just re-exported and do not have their own documentation page.

@jalextowle
Copy link

jalextowle commented Mar 28, 2020

@Gerrit0 I have also been running into this problem when trying to update our document generation to typedoc 0.17.3. Unlike @doberkofler, I am running into this issue when I re-export fields from other packages without changing the name of the field.

The codebase that I am working on is https://github.com/0xProject/0x-mesh, and the specific package that is giving me issues is here: https://github.com/0xProject/0x-mesh/tree/master/packages/browser.

When I try to generate docs with node node_modules/typedoc/bin/typedoc I get the following warning:

Loaded plugin /Users/alextowle/0x-mesh/node_modules/typedoc-plugin-markdown

Using TypeScript 3.8.3 from /Users/alextowle/0x-mesh/node_modules/typescript/lib
Warning: Some names refer to reflections that do not exist.
This can be caused by exporting a reflection only under a different name than it is declared when excludeNotExported is set
or by a plugin removing reflections without removing references. The names that cannot be resolved are:
AcceptedOrderInfo
BigNumber
Config
ContractAddresses
ContractEvent
ERC1155ApprovalForAllEvent
ERC1155TransferBatchEvent
ERC1155TransferSingleEvent
ERC20ApprovalEvent
ERC20TransferEvent
ERC721ApprovalEvent
ERC721ApprovalForAllEvent
ERC721TransferEvent
ExchangeCancelEvent
ExchangeCancelUpToEvent
ExchangeFillEvent
GetOrdersResponse
JsonSchema
LatestBlock
Mesh
OrderEvent
OrderEventEndState
OrderInfo
RejectedOrderInfo
RejectedOrderKind
RejectedOrderStatus
SignedOrder
Stats
SupportedProvider
ValidationResults
Verbosity
WethDepositEvent
WethWithdrawalEvent
No 'out' or 'json' option has been set
The './docs' directory has be set as the output location by default
Rendering [========================================] 100%

Documentation generated at /Users/alextowle/0x-mesh/packages/browser/docs

The documentation that is created for these identifiers is:

[@0x/mesh-browser](README.md) › [Globals](globals.md)

# @0x/mesh-browser

## Index

### Modules

* ["index"]()

## Modules

###  "index"

• **"index"**:

*Defined in [index.ts:1](https://github.com/0xProject/0x-mesh/blob/6495346/packages/browser/src/index.ts#L1)*

###  AcceptedOrderInfo

• **AcceptedOrderInfo**:

###  BigNumber

• **BigNumber**:

###  Config

• **Config**:

###  ContractAddresses

• **ContractAddresses**:

###  ContractEvent

• **ContractEvent**:

###  ERC1155ApprovalForAllEvent

• **ERC1155ApprovalForAllEvent**:

###  ERC1155TransferBatchEvent

• **ERC1155TransferBatchEvent**:

###  ERC1155TransferSingleEvent

• **ERC1155TransferSingleEvent**:

###  ERC20ApprovalEvent

• **ERC20ApprovalEvent**:

###  ERC20TransferEvent

• **ERC20TransferEvent**:

###  ERC721ApprovalEvent

• **ERC721ApprovalEvent**:

###  ERC721ApprovalForAllEvent

• **ERC721ApprovalForAllEvent**:

###  ERC721TransferEvent

• **ERC721TransferEvent**:

###  ExchangeCancelEvent

• **ExchangeCancelEvent**:

###  ExchangeCancelUpToEvent

• **ExchangeCancelUpToEvent**:

###  ExchangeFillEvent

• **ExchangeFillEvent**:

###  GetOrdersResponse

• **GetOrdersResponse**:

###  JsonSchema

• **JsonSchema**:

###  LatestBlock

• **LatestBlock**:

###  Mesh

• **Mesh**:

###  OrderEvent

• **OrderEvent**:

###  OrderEventEndState

• **OrderEventEndState**:

###  OrderInfo

• **OrderInfo**:

###  RejectedOrderInfo

• **RejectedOrderInfo**:

###  RejectedOrderKind

• **RejectedOrderKind**:

###  RejectedOrderStatus

• **RejectedOrderStatus**:

###  SignedOrder

• **SignedOrder**:

###  Stats

• **Stats**:

###  SupportedProvider

• **SupportedProvider**:

###  ValidationResults

• **ValidationResults**:

###  Verbosity

• **Verbosity**:

###  WethDepositEvent

• **WethDepositEvent**:

###  WethWithdrawalEvent

• **WethWithdrawalEvent**:

### `Const` go

• **go**: *Go<>* = new Go()

*Defined in [index.ts:41](https://github.com/0xProject/0x-mesh/blob/6495346/packages/browser/src/index.ts#L41)*

Ideally, the documentation for the @0x/mesh-browser package would be almost identical to the documentation for the @0x/mesh-browser-lite package. The currently generated documentation doesn't even include links, so it's pretty much unusable. In the meantime, I'm going to write a script that will simply copy the documentation from the @0x/mesh-browser-lite package during our document generation and then remove the loadMeshStreamingAsync and loadMeshStreamingWithURLAsync functions from the documentation. Having said this, it would be great if this could be fixed soon, and I'd even be willing to help out if it would be helpful.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Mar 30, 2020

from other packages

Well that explains it. You aren't including that other package in your documentation since declaration files are excluded by default, so it is expected that it wouldn't know what to do with these symbols.

Try:

npx typedoc --includeDeclarations --excludeExternals src/index.ts ./node_modules/@0x/mesh-browser-lite

--includeDeclarations tells TypeDoc to include symbols defined in .d.ts files, --excludeExternals tells TypeDoc to exclude files that are outside of your input files/dirs. If you leave --excludeExternals off, TypeDoc will try to document literally everything (documenting @types/node is not a good idea)

@jalextowle
Copy link

jalextowle commented Mar 30, 2020 via email

@devotox
Copy link

devotox commented Oct 22, 2020

This seems to work for me

  excludeExternals: true,
  excludeNotExported: false,
  includeDeclarations: true,
  ignoreCompilerErrors: false,

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Dec 26, 2020

In 0.20, this warning is completely gone - TypeDoc will remove dangling references.

@Gerrit0 Gerrit0 added this to To do in Version 0.20 via automation Dec 26, 2020
@Gerrit0 Gerrit0 moved this from To do to Done in Version 0.20 Dec 26, 2020
@Gerrit0 Gerrit0 closed this as completed Dec 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion on large effort changes to TypeDoc
Projects
No open projects
Development

No branches or pull requests

4 participants