[mobx-undecorate] Lot's of "Cannot read property 'start' of null" errors

[nejsimon]

Running the mobx-undecorate utility in a fairly large react + typeascript project created using creat-react-app produces lots of these warnings:

Users/simonstrandman/work/.../file.ts Transformation error (Cannot read property 'start' of null) TypeError: Cannot read property 'start' of null at warn (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:405:38) at createConstructor (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:341:17) at NodePath. (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:195:13) at /Users/simonstrandman/work/.../node_modules/mobx-undecorate/node_modules/jscodeshift/src/Collection.js:75:36 at Array.forEach () at Collection.forEach (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/node_modules/jscodeshift/src/Collection.js:74:18) at transform (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:168:37)

This happens in mobx-undecorate/src/undecorate.ts in the function warn():

function warn(msg: string, node: Node) {
    if (process.env.NODE_ENV === "test") {
        return
    }
    const line = lines[node.loc!.start.line - 1] // <-- This line is causing the crash
    const shortline = line.replace(/^\s*/, "")
    console.warn(
        `[mobx:undecorate] ${msg} at (${fileInfo.path}:${node.loc!.start.line}:${
            node.loc!.start.column
        }):\n\t${shortline}\n\t${"^".padStart(
            node.loc!.start.column + 1 - line.indexOf(shortline),
            " "
        )}\n`
    )
}

Turns out node.loc can in fact be null. If I change the above with a null check on node.loc before using it, it runs properly and the actual warnings is outputed to the console. In my case the warning was:

Generated new constructor for class Foo. But since the class does have a base class, it might be needed to revisit the arguments that are passed to super()

[mweststrate]

PR welcome :)

On Mon, Jan 25, 2021 at 11:06 AM Simon Strandman notifications@github.com wrote:

Running the mobx-undecorate utility in a fairly large react + typeascript project created using creat-react-app produces lots of these warnings:

Users/simonstrandman/work/.../file.ts Transformation error (Cannot read property 'start' of null) TypeError: Cannot read property 'start' of null at warn (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:405:38) at createConstructor (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:341:17) at NodePath. (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:195:13) at /Users/simonstrandman/work/.../node_modules/mobx-undecorate/node_modules/jscodeshift/src/Collection.js:75:36 at Array.forEach () at Collection.forEach (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/node_modules/jscodeshift/src/Collection.js:74:18) at transform (/Users/simonstrandman/work/.../node_modules/mobx-undecorate/src/undecorate.ts:168:37)

This happens in mobx-undecorate/src/undecorate.ts in the function warn():

function warn(msg: string, node: Node) { if (process.env.NODE_ENV === "test") { return } const line = lines[node.loc!.start.line - 1] // <-- This line is causing the crash const shortline = line.replace(/^\s*/, "") console.warn( [mobx:undecorate] ${msg} at (${fileInfo.path}:${node.loc!.start.line}:${ node.loc!.start.column }):\n\t${shortline}\n\t${"^".padStart( node.loc!.start.column + 1 - line.indexOf(shortline), " " )}\n )}

Turns out node.loc can in fact be null. If I change the above with a null check on node.loc before using it, it runs properly and the actual warnings is outputed to the console. In my case the warning was:

Generated new constructor for class Foo. But since the class does have a base class, it might be needed to revisit the arguments that are passed to super()

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx/issues/2747, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBCFV7JBJKNC7R3VGNLS3VGCPANCNFSM4WRSKRXA .

[iChenLei]

@nejsimon Any minimal reproduction code snippet (link / github repo / snippet comment)? If you have no time to create PR and I can do it , but need reproduction code to check this problem and add unit test. 😄

[nejsimon]

I can create a PR for the null check i described above. The result would be that the line and col numbers wouldn't be outputted in that case but at least it's better than crashing! :)

Although if it's the case that node.loc really shouldn't be null then I'm not sure what's wrong since I'm not familiar with the code. I'll see if I can create a reproducable test case!

Also I should add that I used the --keepDecorators option.

[xiluo58]

Invalid code:

import React from "react";
import { inject, observer } from "mobx-react";
import { computed } from "mobx";

@inject("store")
@observer
export class ActionItems extends React.Component<any, {}> {
    @computed private get name() {
        return "name";
    }

    public render() {
        return <div>{this.name}</div>;
    }
}

Valid code:

import React from "react";
import { inject, observer } from "mobx-react";
import { computed } from "mobx";

@inject("store")
@observer
export class ActionItems extends React.Component<any, {}> {
    @computed private get name() {
        return "name";
    }

    constructor(props) {
        super(props);
    }

    public render() {
        return <div>{this.name}</div>;
    }
}

This error happens if there is no constructor. I pushed the code to https://github.com/xiluo58/mobx-undecorate-reproduce/tree/main/start-null

[iChenLei]

@xiluo58 Thanks for your reproducation code, I debug these code and find why we encount issue.

Reproduction Code

import React from "react";
import { inject, observer } from "mobx-react";
import { computed } from "mobx";
@inject("store")
@observer
export class ActionItems extends React.Component<any, {}> {
    @computed private get name() {
        return "name";
    }
    public render() {
        return <div>{this.name}</div>;
    }
}

Why the loc will be null

https://github.com/benjamn/recast/blob/master/lib/util.ts#L180-L183

} else if (node.declaration && isExportDeclaration(node)) {
// Nullify .loc information for the child declaration so that we never 👈.   //  why set `loc` as null  💣 
// try to reprint it without also reprinting the export declaration.  
node.declaration.loc = null;

👇 Look the AST structure, our ClassDeclaration's loc isn't null

👇 After recast modify our AST structure.

Summary

mobx-undecorate is built on top of facebook/jscodeshift, and jscodeshift use recast as dependencies. mobx-undecorate use @babel/parser as the AST parser. 👇 codemod script function call stack chian

/ ommited /

https://github.com/facebook/jscodeshift/blob/master/src/core.js#L82

return fromAST(recast.parse(source, options));

/ ommited / https://github.com/benjamn/recast/blob/master/lib/util.ts#L152

export function fixFaultyLocations(node: any, lines: any) {

/ ommited /

node.declaration.loc = null;

btw VScode is very useful for debug js/ts project 😃

100% reproduction code

class comp with export before and class without constructor

solution

Add null check for node.loc and show important warn message enough when loc is null.