Skip to content

[api-extractor] InternalError when analyzing EditorState from draft-js #1001

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

Closed
octogonz opened this issue Dec 20, 2018 · 3 comments
Closed
Labels
bug Something isn't working as intended effort: easy Probably a quick fix. Want to contribute? :-)

Comments

@octogonz
Copy link
Collaborator

@nvh reported an error in issue #949, which on investigation turned out to be a separate issue:

I encountered this error importing the EditorState from draft-js and using it in a class
A minimal reproduction looks like this:

import { EditorState } from "draft-js"
export class Text {
    editorState: EditorState
}

making the editorState private fixes the problem.
Example project:
registerd-as-non-imported.zip
Reproduction steps:
screenshot 2018-11-30 at 16 05 03

The error message was:

The symbol EditorState is being imported after it was already registered as non-imported
@octogonz octogonz added the bug Something isn't working as intended label Dec 20, 2018
@octogonz
Copy link
Collaborator Author

Inside the typings for @types\draft-js\index.d.ts we have this:

export as namespace Draft;
declare namespace Draft {
    namespace Model {
        namespace ImmutableData {
            class EditorState extends Record {
                // ...
            }
        }
    }
}

import EditorState = Draft.Model.ImmutableData.EditorState;

export {
    EditorState
};

Initially I thought this was another instance of the export { x } construct (issue #663), however it seems to be a more mundane problem. In AstSymbolTable.ts, the compiler's isSourceFileFromExternalLibrary() API is returning false for @types\draft-js\index.d.ts which causes a full analysis of this unsupported construct.

        // If the file is from a package that does not support AEDoc, then we process the
        // symbol itself, but we don't attempt to process any parent/children of it.
        const followedSymbolSourceFile: ts.SourceFile = followedSymbol.declarations[0].getSourceFile();
        if (this._program.isSourceFileFromExternalLibrary(followedSymbolSourceFile)) {
          if (!this._packageMetadataManager.isAedocSupportedFor(followedSymbolSourceFile.fileName)) {
            nominal = true;
          }
        }

Whereas if nominal = true then the problematic construct would not be analyzed and the API extraction would be successful.

It seems that the compiler's isSourceFileFromExternalLibrary() doesn't do what I thought it did, since @types/draft-js sure seems like an "external library". Perhaps we need to revert to the earlier approach relying on isAedocSupportedFor --> PackageJsonLookup to determine whether a project belongs to the current project or not... This API was introduced in PR microsoft/TypeScript#12054

@DanielRosenwasser FYI

@octogonz octogonz added help wanted If you're looking to contribute, this issue is a good place to start! effort: easy Probably a quick fix. Want to contribute? :-) labels Dec 20, 2018
@octogonz
Copy link
Collaborator Author

This is working now in my branch for PR #1002

It makes the analysis accurate so that we can simply test whether followAliasesResult.astImport is defined or not. If so, then the symbol is from an external package.

@octogonz octogonz removed the help wanted If you're looking to contribute, this issue is a good place to start! label Dec 20, 2018
@octogonz
Copy link
Collaborator Author

Fixed in #1002

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended effort: easy Probably a quick fix. Want to contribute? :-)
Projects
None yet
Development

No branches or pull requests

1 participant