Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

Export transformBefore instead of transform #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

difosfor
Copy link

When transform is exported it is aliased to transformAfter. I think using transformBefore is intended (based on comment), makes more sense and will resolve an indentation issue I'm running into now

When transform is exported it is aliased to transformAfter. I think using transformBefore is intended (based on comment), makes more sense and will resolve an indentation issue I'm running into now
@difosfor
Copy link
Author

Hmm.. after testing I am now seeing another issue. Also it looks like the tests are failing. Perhaps this plugin is better off with transformAfter after all, but requires some other kind of fix?

Input:

var p=[1],q=2;

Expected:

var p = [
    1
];
var q = 2;

Actual:

var p=[
    1
];
var q=2;

PS: The original issue I had when transform(After) was being used was incorrect indentation:

var p = [
        1
    ];
var q = 2;

If I pass that output through esformatter again it will result in properly indented code. But I would rather have esformatter do things right at once.

I am testing with the following config at the moment:

{
    "preset": "default",
    "indent": {
        "value": "\t"
    },
    "lineBreak": {
        "before": {
            "ArrayExpressionClosing": 1,
            "BlockComment": 2,
            "CommentGroup": 2,
            "ReturnStatement": 2
        },
        "after": {
            "ArrayExpressionComma": 1,
            "ArrayExpressionOpening": 1,
            "ClassDeclarationClosingBrace": 2,
            "FunctionDeclarationClosingBrace": 2
        }
    },
    "plugins": [
        "esformatter-braces",
        "esformatter-dot-notation",
        "esformatter-ignore",
        "esformatter-quotes",
        "esformatter-remove-trailing-commas",
        "esformatter-semicolons",
        "esformatter-var-each"
    ],
    "quotes": {
        "type": "single"
    }
}

@twolfson
Copy link
Owner

It does seem like we used the wrong transform -- not sure why I documented one and didn't use the other. Unfortunately, looking at the latest docs -- it seems like esformatter wants us to move off of using their AST and generate our own via stringBefore:

https://github.com/millermedeiros/esformatter/blob/v0.8.1/doc/plugins.md#transformbeforeast

That definitely makes sense -- keeping track of nodes/tokens was a PITA for this library =/

@twolfson
Copy link
Owner

Another option might be to explore using tokenBefore but then we will be relying on str === 'var' instead of VariableDeclaration so it might get buggy =/

@difosfor
Copy link
Author

Hmm.. sounds more complicated then I have time for right now I'm afraid.

As I've mentioned in #5 I have found a reasonable workaround (indent MultipleVariableDeclaration 0), so I'm good for now. Maybe you could mention that workaround in your documentation until you update/rewrite your code?

At any rate you can close this pull request if you like.

Thanks!

Peter

@twolfson
Copy link
Owner

I'm not too free to look into this either but I think the documentation solution might be a nice middle ground for now

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants