487 add eslint

[pivaszbs]

add eslint plugin #487

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
reatom	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Mar 6, 2023 at 11:16AM (UTC)

[vovanezha]

I'd define all possible messages here to make a code cleaner:

Suggested change

	meta: {
	type: 'suggestion',
	docs: {
	description: "Add name for every action call"
	},
	fixable: 'code'
	},
	meta: {
	type: 'suggestion',
	docs: {
	description: "Add name for every action call"
	},
	messages: {
	noname: /* message */,
	invalidName: /* message */
	},
	fixable: 'code'
	},

What do you think?

[pivaszbs]

I think it's not good idea as it's not easy to pass variables inside)

But put messages as separate functions at the top is great

[vovanezha]

why not easy? with a structure above you can pass variables like this:

    context.report({
                messageId: "noname",
                data: { varName: varName },
                ...
              });

[pivaszbs]

Is there any advantages on such a solution in comparison with simple function?

message: noname(d.id.name),

[vovanezha]

1. we use builtin tools
2. all rule configuration in one place

[pivaszbs]

anyway it's not typed and a bit harder to read)) (for me)

[vovanezha]

Maybe we also have to check if the current declaration is imported from @reatom?

[pivaszbs]

Maybe, but I think it's not easy task (mb I am wrong, I didn't test it on files actually)

[vovanezha]

as far as I remember, we could do it like this:

  create: function (context) {
  let importedFromReatom = false;
  
  return {
    ImportSpecifier(node) {
        const imported = node.imported.name
        const from = node.parent.source.value;
        if (from.startsWith('@reatom') && imported === 'action') {
          importedFromReatom = true;
        }
      },
     ...
  }

(i didn't test this code on your pr, just took it from one of my project :))

[pivaszbs]

Make sense)

[pivaszbs]

Also I have problems with test runner @artalar it doesn't show anything at all in my local runs (throw errors if any, but clear term if no errors)
Mb it's cause of eslint testRunner. Could you help please?

[artalar]

Why does it needed?

[pivaszbs]

good part is eslint rule for eslint plugins)

[artalar]

Suggested change

	...,
	//...

Make it easy to copy-pase

[artalar]

Suggested change

	"last 4 chrome versions"
	"node 16"

[artalar]

It is not used

Suggested change

	"dependencies": {
	"@reatom/core": ">=3.1.0"
	},

[artalar]

We should handle all reatom* methods, it doesn't metter what import it from.

[pivaszbs]

Well, again, no democracy((
Ok, mb it's better

[pivaszbs]

Also I have problems with test runner @artalar it doesn't show anything at all in my local runs (throw errors if any, but clear term if no errors) Mb it's cause of eslint testRunner. Could you help please?

@pivaszbs you run npm run test from the root? Attach screenshot with terminal output pls.

In root

In package

[artalar]

@pivaszbs pull from the mester and retry tests.

How do you think, we are ready to merge this?

[pivaszbs]

@artalar yeah we are ready :)

[artalar]

@pivaszbs there are still three prefux in the code, fix it pls :)

[artalar]

Thank you!