node-red / nrlint

Node-RED Flow Linter
Apache License 2.0
36 stars 15 forks source link

Node red tab hogging the processor in the browser #36

Closed colinl closed 1 year ago

colinl commented 2 years ago

Current Behavior

Having upgraded from nrlint 1.0.2 to 1.1.0 the node-red tab is hogging the browser on Firefox and Chrome. Rollback to 1.0.2 restores normal operation. Offending flow file shared privately.

Expected Behavior

No response

Steps To Reproduce

No response

Example flow

paste your flow here

Environment

My-Random-Thoughts commented 2 years ago

I thought it was just me. Firefox 103 on Linux, an entire CPU core is maxed out

As an additional, it doesn't actually performing any linting either

colinl commented 2 years ago

I suspect the reason that it isn't doing any linting is because the process doing the linting is looping permanently trying, and failing, to parse a piece of code somewhere. @My-Random-Thoughts if you have a short flow showing the problem then that might be helpful.

My-Random-Thoughts commented 2 years ago

I have a test environment and removed all my flows and tabs but it was still maxing out. I can send someone the flow.json file, but I not sure it's a flow specific issue

colinl commented 2 years ago

For me it was ok with an empty flows file (I think). What happens if you delete the flows file? In fact rename it rather than deleting it. Make sure the startup log then says it is creating a new flows file.

My-Random-Thoughts commented 2 years ago

Um, maybe it is the flow file. I renamed my current, restarted the docker container and it created a new flow.json file. I created a couple of test flows, all looking OK I imported some of the example flows - didn't know they were there! - All good Imported my original file (via copy/paste of the json) and it maxed out

knolleary commented 2 years ago

@colinl thank you for sharing your flow with my on the forum.

I can recreate the issue and have narrowed it down to the function-eslint rule.

In fact, I've narrowed it down a bit further than that...

First I identified one node in your flow that would cause the 100% CPU if present. It contains the following code:

const panels = msg.payload.panels
for (let i = 0; i < panels.length; i++) {
    const targets = panels[i].targets
    for (let j = 0; j < targets.length; j++) {
        node.send({ payload: `${targets[j].rawSql}\n` })
    }
}

return null;

I then narrowed it down to one line in that code:

        node.send({ payload: `${targets[j].rawSql}\n` })

I then narrowed it down to the \n in that line.

Quite why the presence of \n in that line is causing ESLint to get stuck, I have no idea. But its a start.

My-Random-Thoughts commented 2 years ago

Wow, I can confirm this. In my test environment, I have one single \n string in a function. I removed it, deployed and refreshed the page. All good now.

Very bizarre!

knolleary commented 2 years ago

Have raised an upstream issue with eslint4b - the library we use to allow eslint to run in the browser.

Curiously, eslint@7.32.0 (the same version as eslint4b we have in nrlint) does not have a problem with this code.

https://github.com/mysticatea/eslint4b/issues/17

knolleary commented 1 year ago

Given the upstream project appears to have been abandoned, we are going to have to rethink how we use eslint in the browser.

We might have to fall back to doing it in the runtime side only - with some api in place for the browser linter to call it. That isn't ideal as currently all of the rules are written without knowledge of where they are running.