pmndrs / eslint-plugin-valtio

An eslint plugin for better valtio experience
MIT License
78 stars 6 forks source link

Bug with reading id #10

Closed nosferatu500 closed 3 years ago

nosferatu500 commented 3 years ago

Hi, I have been using valtio for several months now and decided to try using this plugin today and this is what I ran into.

Code:

class ActionDispatcher {
    modalRenderer: IModalRenderer | null = null;

    openModal(modal: ModalComponent, customData?: unknown): Promise<void> {
        return new Promise<void>((resolve, reject) => {
            // FIXME: error is here
            if (this.modalRenderer) { 
                this.modalRenderer(modal, resolve, customData);
            }
        });
    }
}

Error:

ESLint: 7.32.0

TypeError: Cannot read properties of null (reading 'id')
Occurred while linting /myproject/src/actions/ActionDispatcher.tsx:15
    at ThisExpression (/node_modules/eslint-plugin-valtio/index.js:366:30)
    at /node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/node_modules/eslint/lib/linter/node-event-generator.js:293:26)
    at NodeEventGenerator.applySelectors (/node_modules/eslint/lib/linter/node-event-generator.js:322:22)
    at NodeEventGenerator.enterNode (/node_modules/eslint/lib/linter/node-event-generator.js:336:14)
    at CodePathAnalyzer.enterNode (/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23)
    at /node_modules/eslint/lib/linter/linter.js:960:32
    at Array.forEach (<anonymous>)
dai-shi commented 3 years ago

Thanks for reporting! We'd admit the plugin is not yet very mature and your bug report is appreciated. @Aslemammad can look into it?

nosferatu500 commented 3 years ago

One more question.

I see this rule valtio / state-snapshot-rule in the description. Is that just one rule? Do you have a list of rules and descriptions for each?

dai-shi commented 3 years ago

Good point. #8 is now added. @barelyhuman Would you please update readme?

barelyhuman commented 3 years ago

@dai-shi https://github.com/pmndrs/eslint-plugin-valtio/pull/11 Done

Aslemammad commented 3 years ago

Oh, I cannot look into this nowadays soon, but I'll do it later. If @barelyhuman's able to handle this, then that'd be excellent!

barelyhuman commented 3 years ago

@Aslemammad I'll see what I can do , I barely know what's causing the issue so the debugging may take a bit longer than needed.

barelyhuman commented 3 years ago

@nosferatu500 Could you confirm if the below modified package works as intended?

yarn add https://pkg.csb.dev/pmndrs/eslint-plugin-valtio/commit/a9309e8b/eslint-plugin-valtio
npm i https://pkg.csb.dev/pmndrs/eslint-plugin-valtio/commit/a9309e8b/eslint-plugin-valtio

@dai-shi https://github.com/pmndrs/eslint-plugin-valtio/pull/12 for the changes made

nosferatu500 commented 3 years ago

@barelyhuman

Yes, I just checked it. The problem is gone. Thanks!

dai-shi commented 3 years ago

12 is merged and published https://www.npmjs.com/package/eslint-plugin-valtio/v/0.4.1.

barelyhuman commented 3 years ago

@nosferatu500 you can now use the official package

npm i eslint-plugin-valtio@^0.4.1
yarn add eslint-plugin-valtio@^0.4.1