Meteor 3.0 - async functions eslint plugin

I just interlink the 2 posts as they seem to be related: How did I migrate our app to new Async API (Meteor 2.8+) - #26 by minhna

Hi @batist , did you ever get around to publishing your eslint plugin?

Maybe a gist or something would be enough for the time being?

I’m looking at collecting resources for the transition at the moment, so the more the merrier! :smiley:

That would be very helpful, we had this in our backlog, we could use yours and provide PRs if needed

1 Like

We also have this eslint config package

The idea was to setup the rules for Meteor 3 there as well

@batist that would be awesome. I created a Meteor package for detection of non-async but it’s only working at runtime. A static analysis with ESLINT would be the perfect counterpart for a holistic analysis approach!

Edit: this would also be beneficial for CI workflows that check for 3.0-readiness

2 Likes

I’ve started work on this today.

First, I’ve created a simple new rule to check for Meteor.user() calls. See the result in the IDE:

What do you think?

Suggestions? Changes? Any feedback is appreciated.

1 Like

Hi Filipe,

are you aware of this content in a previous post?

module.exports = {
	type: 'suggestion',
	docs: {
		description: 'Migrate away from fibers'
	},
	create(context) {
		return {
			'MemberExpression > Identifier': function(node) {
				//createIndex
				if (node.name.toLowerCase() == 'createindex') {
					return context.report({
						node,
						message: 'use createIndexAsync instead',
						data: node
					});
				}
				//insert
				if (node.name.toLowerCase() == 'insert') {
					return context.report({
						node,
						message: 'use insertAsync instead',
						data: node
					});
				}
				//findOne
				if (node.name.toLowerCase() == 'findone') {
					return context.report({
						node,
						message: 'use findOneAsync instead',
						data: node
					});
				}
				//update
				if (node.name.toLowerCase() == 'update') {
					return context.report({
						node,
						message: 'use updateAsync instead',
						data: node
					});
				}
				//remove
				if (node.name.toLowerCase() == 'remove') {
					return context.report({
						node,
						message: 'use removeAsync instead',
						data: node
					});
				}
				//count
				if (node.name.toLowerCase() == 'count') {
					return context.report({
						node,
						message: 'use countAsync instead',
						data: node
					});
				}
				//fetch
				if (node.name.toLowerCase() == 'fetch') {
					return context.report({
						node,
						message: 'use fetchAsync instead',
						data: node
					});
				}
				//map and foreach are a bit dangerous, no?

				//validatedMethod call
				if (node.name.toLowerCase() == 'call') {
					return context.report({
						node,
						message: 'use the async mixin',
						data: node
					});
				}

				
				//2.9 changes
				if (node.name.toLowerCase() == 'user') {
					return context.report({
						node,
						message: 'use userAsync instead',
						data: node
					});
				}
				return null;
			},
		};
	},
};

Hey, I don’t think so.

I was trying to find the content you posted here by clicking on the links from this post but I didn’t find it.

Is it in a PR against meteor-eslint-plugin? That is what I’m planning to do, so if that is the same, great, we can join efforts.

Is this version detecting if the code is in the server or the client?

Thanks.

2 days ago in this thread I interlinked the other thread that contains the discussion about eslint. More specifically, this is the comment that has the repo:

1 Like

We currently have this. (We’re also trying to add some fixer logic to speed up updating the code base.)

const invalidFunctionNamesWithoutArguments = [
    'fetch',
    'count',
];
const invalidFunctionNamesWithArguments = [
    'findOne',
    'insert',
    'update',
    'upsert',
    'remove',
    'createIndex',
];
/* forEach, map */

function createError(context, node) {

    context.report({
        node: node.parent,
        message: 'Should use Meteor async calls',
        *fix(fixer) {
            if (node.parent.parent.type.endsWith('Expression')) {
                yield fixer.insertTextBefore(node.parent, '(await ');
                yield fixer.insertTextAfter(node.property, 'Async');
                yield fixer.insertTextAfter(node.parent, ')');
            }
            else {
                yield fixer.insertTextBefore(node.parent, 'await ');
                yield fixer.insertTextAfter(node.property, 'Async');
            }
        },
    });
}

module.exports = {
    meta: {
        type: 'problem',
        docs: {
            description: 'Detect sync Meteor calls',
            recommended: true,
        },
        fixable: 'code',
    },

    create: function(context) {

        return {
            'MemberExpression': function(node) {
                if (node.property && node.property.type === 'Identifier') {
                    if (invalidFunctionNamesWithoutArguments.includes(node.property.name) && node.parent.arguments?.length === 0) {
                        createError(context, node);
                    }
                    else if (invalidFunctionNamesWithArguments.includes(node.property.name) && node.parent.arguments?.length > 0) {
                        createError(context, node);
                    }
                }
            },
        };
    },
};

How are you handling client vs server code?

I think this is the part that I’m missing.

All these functions and methods are still valid and should be used in many cases in the client.

Thank you.

I just checked it now, but I see two problems:

  • It should be a PR in the Meteor ESLint plugin
  • We should try to differentiate between server and client. All that validation is wrong for the client. These methods are still valid for client-only code. Maybe you want to avoid it in some cases and use async in others, but we definitely should not consider it simply an error.

I want to find a way to get this information from Meteor, but I think that will be hard or impossible inside the eslint plugin as the Meteor compiler decides it on the fly. Maybe for projects using “mainModule” (everybody should be using it), it would be more straightforward.

@zodern, any insights on how to achieve that inside the eslint-plugin?

Hi, I think this could be / should be two different sets of rules actually:

  1. One for helping people migrate - Basically flagging everything sync as errors so people can quickly scan their code for things they might have missed.

Maybe this wouldn’t have to go into Meteor for now, a Gist or released as a separate Eslint config to add to your own would be enough?

2nd: One maybe with warnings which could go into “Meteor Proper” for the future, which could potentially mark Synchronous code as “warning”?

Hi Daniel, we can do these by grouping the rules into “recommended”, “meteor-3-migration”, etc.

The rules are the rules, and then we can create shortcuts for usage in groups. Do you agree?

So, we focus on having all the rules in the same place.

More discussions about Client vs Server here on Slack

The basic rules to find all files used on the server isn’t too complicated:

  1. Find eagerly loaded files
    a. check if the server has a main file
    b. if not, look for any js files outside of folders named imports, client, packages, or folders that start with a .
  2. Parse each of the eagerly loaded files and scan for static imports, dynamic imports, and require calls
  3. Resolve each of those imports, filter out the ones that resolve to a file in a client folder or to a npm or Meteor package. Then parse those files and find the imports…
  4. Repeat step 3 until there are no more files

A few years ago I wrote a tool to find all client files for a company. They gave me permission to open source it, but I have never got around to finishing that. If you think the code would be helpful, I can find it.

Sure, thank you.

Once you share this code, I’ll work to adapt it to work with the Meteor ESLint plugin, and then we can have more precise rules.

Thanks @zodern

What is the current status of this? Do we have a standard linter plugin to help with migration?

1 Like

We use the Quave ESLint plugin in all our projects and our clients’ projects.

Zodern shared some code, but it wasn’t complete. We expanded it to fully support the correct linting of server-side code.

2 Likes

Thanks @filipenevola