Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support dynamic import #1197

Closed
renatofdds opened this issue May 23, 2017 · 14 comments
Closed

Support dynamic import #1197

renatofdds opened this issue May 23, 2017 · 14 comments

Comments

@renatofdds
Copy link

renatofdds commented May 23, 2017

Input

The code looked like this before beautification:

frontend = Async(() => import('../frontend').then(m => m.default      ))

Expected Output

The code should have looked like this after beautification:

frontend = Async(() => import('../frontend').then(m => m.default))

Actual Output

The code actually looked like this after beautification:

frontend = Async(() =>
        import('../frontend').then(m => m.default))

Environment

OS: Linux 4.10, node 7.10

Settings

Example:

{
    "brace_style": "collapse,preserve-inline",
    "break_chained_methods": false,
    "end_with_newline": false,
    "eol": "\n",
    "e4x": true,
    "eval_code": false,
    "indent_char": " ",
    "indent_level": 0,
    "indent_with_tabs": false,
    "jslint_happy": false,
    "keep_array_indentation": false,
    "keep_function_indentation": false,
    "max_preserve_newlines": 2,
    "preserve_newlines": true,
    "selector_separator_newline": false,
    "space_after_anon_function": false,
    "space_before_conditional": true,
    "unescape_strings": false,
    "wrap_attributes": "auto",
    "wrap_attributes_indent_size": 4,
    "wrap_line_length": 0,
    "indent_size": 4
}

TC39 Proposal

@briefguo
Copy link

the same need

@ZzZombo
Copy link

ZzZombo commented Jul 25, 2018

Please? This is much needed for me. Kills the the point of using the formatter for me.

@laggingreflex
Copy link

In the meanwhile if you can use async/await you can use async () => await import() (#1433)

@bitwiseman bitwiseman added this to the v1.8.x milestone Jul 25, 2018
@bitwiseman
Copy link
Member

@laggingreflex Thanks for the work around. That will help with with implementing a fix.

@bitwiseman
Copy link
Member

Note, in the original input there is a space. It is preserved in the output in rc6, exactly as you had above.

In rc6 if you pass:

frontend = Async(() => import('../frontend').then(m => m.default))

You get:

frontend = Async(() =>
    import('../frontend').then(m => m.default))

Which is also valid.

@ZzZombo
Copy link

ZzZombo commented Aug 13, 2018

What??? You gotta be kidding, man. No way you dare to label this as resolved simply because the resulting output is valid syntax.

@bitwiseman
Copy link
Member

@ZzZombo
Are you joking or not? I can't tell.
The input is valid, that output is also valid and has the same meaning as the input. How is that wrong?

@ZzZombo
Copy link

ZzZombo commented Aug 13, 2018

Oh my god, you weren't kidding, holy shit. It's so fucking stupid that I even have to explain to you why it's wrong that a code formatter makes formatting worse, and BTW, nothing was even "fixed" because nothing was changed. The described – and wrong – behavior remained the same ever since. I mean, if you still don't get it, one can write code with trailing whitespace, it's valid JS still, yea, but wrong nonetheless, so that's why code formatters exist in the first place, to fix such issues, not make even more of them, especially where were none to begin with.

@bitwiseman
Copy link
Member

@renatofdds
Did you mean to have a space between import and ( in your input but not in your output?

@renatofdds
Copy link
Author

@bitwiseman yes.

@bitwiseman
Copy link
Member

@renatofdds
Just to be clear, I thought dynamic import was specifically import( (without the space). So, you want to beautifier to remove the space to make a method call into a dynamic import call when it wasn't such in the input?

@renatofdds
Copy link
Author

@bitwiseman I am sorry, i just used it as an example while writing this issue without thinking too much about it. I think too that beautifier shouldn't handle that space since it's a syntax error. However, the initial issue could be written as:

Input

The code looked like this before beautification:

frontend = Async(() => import('../frontend').then(m => m.default      ))

Expected Output

The code should have looked like this after beautification:

frontend = Async(() => import('../frontend').then(m => m.default))

Actual Output

The code actually looked like this after beautification:

frontend = Async(() =>
        import('../frontend').then(m => m.default))

I no longer use beautifier so i have no way to tell if this issue is already fixed, i am sorry. If it is already fixed please go ahead and close this.

Thank you very much!

@bitwiseman
Copy link
Member

No, it is not fixed, but thanks for taking the time to give cleaned up info. I've updated the original description.

@bitwiseman
Copy link
Member

Implementation note: import() needs to be treated by the beautifier as a not a "line-starter". Changing it from reserved to word when it is immediately followed by ( without whitespace would work (similar to get/set at the top of handle_word).

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

No branches or pull requests

6 participants