Multi-line ES6 import

[supersteves]

import {
    something,
    somethingElse,
    inFactAVeryLongList,
    ofLotsOfThings,
    whichIsUglyOnOneLine
} from "./MyAllPurposeUtil";

This doesn't get picked up by the regex import scanning in Closure Compiler, because it assumes each statement is one per line:

https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/deps/JsFileParser.java#L41

Probably tough to fix. But also something that can be hard to diagnose.

Easy workaround:

import {
    ...
} from "./MyAllPurposeUtil";
import "./MyAllPurposeUtil"; // tell CC about this dep

[ChadKillingsworth]

Regex parsing for ES6 modules is causing problems in other areas. I'm going to discuss it at the meeting this afternoon to figure out options.

[MatrixFrog]

@shicks am I remembering correctly that we decided to make this a one-line regex instead of multi-line for simplicity/speed? I think the style guide says not to wrap the line but projects outside of Google are likely not to follow our style guide.

[ChadKillingsworth]

@shicks Is this just when using the deps package? It looks like the ProcessEs6Modules properly adds the deps information.

[supersteves]

That's right, I only came across this when relying upon the deps package result. It correctly builds providing the deps haven't been dropped.

[shicks]

TBH, I really don't like the empty import workaround. The issue is indeed with the referred-based scanning in the deps package, which does need to be fast, since large projects with many framework dependencies can sometimes load many thousands of files that need to be pruned without a full parse. Given those requirements, I'm not sure what the best solution is. As Tyler mentioned, internally we have a style rule that imports can't break multiple lines - they're allowed to (and must) be as long as they need to be. One option would be to read additional lines if a line starts with import and doesn't end with a semicolon. It's a little brittle, but so's the whole thing already. But I'm also not sure how easy that would be given the JsFileLineParser framework really wants to only operate on one line at a time. We could also change the latter to just automatically read additional lines until one ends with `;\s*($|//)`. That would allow things down a little, but hopefully not a whole lot.

…

On Nov 29, 2016 5:51 AM, "Steve Spencer" ***@***.***> wrote: That's right, I only came across this when relying upon the deps package result. It correctly builds providing the deps haven't been dropped. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2175 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIyG9R6PA8wE0M7pUWNTYz3bYglgHaAks5rDC35gaJpZM4K-FBw> .

[supersteves]

Alternative workaround:

import { something, somethingElse, inFactAVeryLongList } from "./MyAllPurposeUtil";
import { ofLotsOfThings, whichIsUglyOnOneLine } from "./MyAllPurposeUtil";

[supersteves]

Perhaps the deps parser could detect an unclosed import statement and emit a warning.

[ChadKillingsworth]

To my knowledge, the deps package has little use outside of Google. Changing it is also difficult for an external contributor for all the reasons @shicks listed.

I looked into what would be required to change this and don't have the bandwidth to tackle it. Since this doesn't affect full compilations in any way, It's not a priority for me right now.

[timdawborn]

I realise this is a stale ticket now, but this worked with STRICT dependency mode for a while and then stopped working again. The v20170521 release was the latest working release. It'd be really nice to not have to change our entire codebase to not use multi-line imports just so we can use a newer Closure release :/

Repro:

$ cat foo.js
export const FOO = 1;
export const BAR = 2;

$ cat entry-single.js
import { BAR, FOO } from './foo.js';

export const BAZ = FOO + BAR;

$ cat entry-multi.js
import {
  BAR,
  FOO,
} from './foo.js';

export const BAZ = FOO + BAR;

$ for j in $(find . -name '*.jar' | sort); do echo $j; java -jar $j --js foo.js --js entry-single.js --entry_point entry-single.js --js_output_file /dev/null --dependency_mode STRICT; done
./v20170423.jar
./v20170521.jar
./v20170626.jar
./v20170806.jar
./v20170910.jar

$ for j in $(find . -name '*.jar' | sort); do echo $j; java -jar $j --js foo.js --js entry-multi.js --entry_point entry-multi.js --js_output_file /dev/null --dependency_mode STRICT; done
./v20170423.jar
./v20170521.jar
./v20170626.jar
entry-multi.js:1: ERROR - required "module$foo" namespace never provided
import {
^^^^^^^^

1 error(s), 0 warning(s)
./v20170806.jar
entry-multi.js:1: ERROR - required "module$foo" namespace never provided
import {
^^^^^^^^

1 error(s), 0 warning(s)
./v20170910.jar
entry-multi.js:1: ERROR - required "module$foo" namespace never provided
import {
^^^^^^^^

1 error(s), 0 warning(s)

[ChadKillingsworth]

This was fixed by #2641