generate JsDoc api.json fails on MemberExpressions with Literals

[killerfurbel]

Expected Behavior

Running the build process to generate the api.json without errors:

    await builder.build({
        tree: tree,
        destPath: params.destPath,
        cleanDest: false,
        jsdoc: true,
        buildDependencies: true
    });

Current Behavior

.../node_modules/@ui5/builder/lib/processors/jsdoc/lib/ui5/plugin.js:294
       path: getObjectName(valueNode).split('.').slice(1).join('.') // TODO chaining if local has path
                                     ^
TypeError: Cannot read property 'split' of null

(Exception is generated here: https://github.com/SAP/ui5-builder/blob/master/lib/processors/jsdoc/lib/ui5/plugin.js#L294)

Steps to Reproduce the Issue

When you use babel-plugin-transform-modules-ui5 for TypeScript coding, you will import library enums like this:

import { FlexRendertype, ButtonType } from "sap/m/library";

Babel will transform that into an sap.ui.define() call with "sap/m/library" which produces a variable sap_m_library. After that, it will generate the enums as follows:

  const FlexRendertype = sap_m_library["FlexRendertype"];
  const ButtonType = sap_m_library["ButtonType"];

To reproduce, you only need to use this syntax, no TypeScript or other transformations are necessary.

The problem is gone when you rewrite the code as follows:

  const FlexRendertype = sap_m_library.FlexRendertype;
  const ButtonType = sap_m_library.ButtonType;

The reason for this is that in the getObjectName() function, only specific AST nodes are parsed: https://github.com/SAP/ui5-builder/blob/master/lib/processors/jsdoc/lib/ui5/plugin.js#L523

function getObjectName(node) {
	if ( node.type === Syntax.MemberExpression && !node.computed && node.property.type === Syntax.Identifier ) {
		const prefix = getObjectName(node.object);
		return prefix ? prefix + "." + node.property.name : null;
	} else if ( node.type === Syntax.Identifier ) {
		return /* scope[node.name] ? scope[node.name] : */ node.name;
	} else {
		return null;
	}
}

The error occurs if the node.type is MemberExpression, but node.computed is true and node.property.type is Literal.

This is how the AST looks like in the error case:

My suggestion is to add another else if clause which covers this case:

function getObjectName (node) {
    if( node.type === Syntax.MemberExpression && !node.computed && node.property.type === Syntax.Identifier ) {
        const prefix = getObjectName(node.object);
        return prefix ? prefix + "." + node.property.name : null;
    } else if( node.type === Syntax.MemberExpression && node.computed && node.property.type === Syntax.Literal ) {
        const prefix = getObjectName(node.object);
        return prefix ? prefix + "." + node.property.value : null;
    } else if( node.type === Syntax.Identifier ) {
        return /* scope[node.name] ? scope[node.name] : */ node.name;
    } else {
        return null;
    }
}

Context

• UI5 Module Version (output of ui5 --version when using the CLI): 2.12.1
• Node.js Version: 10.22.1
• OS/Platform: OpenSUSE 15.2

Solution

I could also create a pull request if you don't have any better idea?

[RandomByte]

Thank you for your excellent analysis @killerfurbel!

I was able to reproduce the issue with the OpenUI5 library sap.f by changing this line to read

var PageBackgroundDesign = mLibrary["PageBackgroundDesign"];

Executing ui5 build jsdoc or grunt docs --libs=sap.f results in the reported error.

I can also confirm that your proposed patch resolves the issue. I will prepare a PR for that. CC: @codeworrior

Thanks again!

[codeworrior]

Without further analysis, the second branch IMO is only safe for a string literal whose value qualifies as an identifier.

As long as the result of getObjectName is only used to identify certain things, the implicit conversion to string is enough. But if calling code assumes that the object name can be used "as is" in a JavaScript expression, then we should limit the change to string literals that are valid property identifiers.

E.g. the export property in the api.json should only contain a chain of identifiers.

[dinialo]

I have the same issue. any plan to fix this? any workaround?
Thanks and regards.
cc @RandomByte

[dinialo]

As workaround (works for me):
If you export directly function(s) (not encapsulated in a class), then do not import directly but import it/them like:

import * as Utils from "file/where/function/is/exported";

[flovogt]

Thanks @killerfurbel for your detailed analysis. I have retested the scenario with UI5 Tooling V3 and no error occurs. However a warning is logged to the console WARNING: did not understand default value 'PageBackgroundDesign.Standard' of property 'backgroundDesign', falling back to source. We will continue to analyze the issue and post any updates here.