Not fully working...

[madasebrof]

Very cool concept!

I have a large Elm project (>15,000 LOC). It semi-works, but some functions are not showing up as having any external references when they do.

Not sure if this is a time-out thing or what.

(I have an 1800 line Update file, for example, and many functions that are used in that file are showing up as having 0 external references.)

Seems to work for the smaller files.

Let me know if there is something I can do to help debug...

[mbuscemi]

I would love to check this out with some debugging functionality turned on. Is your project available on GitHub? If so, please include the name of the 1,800 line file and some files with functions that reference that file's functions.

The largest project I've tested Elm Lens on (in terms of loc) was about 11k lines, and that seemed to work fine. The large file could be an issue... or something else about those references could be the cause.

[mbuscemi]

@madasebrof Just checking in to see if it would be possible for me to get access to this project to find out what's going on. Or perhaps you've resolved the issue? Please let me know.

[madasebrof]

Hey, sorry for the delay. Unfortunately, not an open source project. Yes, still having the error, let me know what steps you'd take to debug (if not to elaborate!) if you had access and I will try to debug.

Again, a cool idea, especially from large Elm projects!

[mbuscemi]

Here's an Ellie I made: https://ellie-app.com/fwqbhnLQZa1/1. It's a thin wrapper over elm-syntax, which is the AST Elm Lens uses. Compile this, and you should get a text box on the right side of the screen. You can copy and paste Elm source code into it, and elm-syntax will run against it. It will either parse and you'll see a ton of AST markup on the screen, or the parsing will fail and you'll get a red error message.

From your description, it sounds as though perhaps some of your files aren't getting parsed. So, try running your files through this, and if you don't get any error messages, then we'll have to try another tactic.

Also, are you using elm-format?

[madasebrof]

Cool, will try. Yes, using elm-format in Atom, I can guarantee there are no errors in any files! :)

...

Okay, all good, no errors (took a few seconds to parse the 1800 line file!)

[madasebrof]

Yeah, still not fully working. It does seem to be partially working, in that it will show the correct count of internal references to functions, but often the external count in incorrect. (e.g. shows up as zero when there are, in fact, external references to functions in files which import the function in question).

Would have to understand the "tree-shaking" logic to help debug further, e.g. how does elm-lens know what files to look for when it does its reference checking? Because both files in question are being parsed and updated by elm-lens (e.g. the files with references as well as the file that contains the original functions).

You can see from the attached screenshot why I say it's "semi-working". For example, it's picking up that subscriptions is used externally, but not sendFileToPoloS3 or sendFileToPoloSnap, even though you can clearly see the function being used in the update funtion on the right.

[madasebrof]

Okay, maybe this will help you debug.

Checkout:

https://github.com/madasebrof/elm-taco-donut

Load up with elm-lens.

Look under src/B_Message.elm

Notice that the type Route shows up as having 0 external references, yet it has 3 (it is used in A-model.elm, B_messages.elm, and E_init.elm).

Same for HomeMsg, which is used by Updates/Home.elm

DonutMsg shows up as having 1 external reference instead of 2, etc.

Best of luck!

[mbuscemi]

Okay, this is a tricky one.

In order to count external references, Elm Lens has to be able to figure out they're external. In order to figure that out, it is reading through all the imports so that when it goes through the expressions, it can tell which come from outside and which come from within. Using (..) in imports causes a problem. Elm Lens parses files in whatever order Atom decides to hand them over in. So, for example, when E_Init.elm is parsed, there is no guarantee that the full exposing list for B_Message.elm is available (it might be later in the queue). And so, it is assuming that Route is an internal reference, because it can't find it amongst the exposings.

I can confirm that this is the issue, because when I change the import statement to specify specific imports, the counting works correctly.

I will have to think about how I want to handle this. I think I would like to come up with some kind of fix, although it is worth noting that best practice dictates that you always specify your imports explicitly. Elm Analyse will warn you against using (..).

[madasebrof]

Was thinking that might be it.

So, it's not doing "tree shaking", e.g. looking at the entry point, then follow from there like the compiler does. Make sense, that would be a lot of work.

Okay, fair enough.

Yes, typically, you'd use explicit importing. It's just that when you want to import all of the functions in a file it makes the files super ugly and adds much boilerplate, not to mention multiple places to update in a project when you add more functions.

(I generally prefer to go the route of not exporting functions in the first place and use (..) when importing vs. explicitly importing them, but that's just me.)

Anyway, good to know, I still may do that (explicit importing) as a "final pass" as I do think it's nice to see reference checking on function usage.

Rock on.

[mbuscemi]

I've done a lot of thinking about how I want to solve this, and here's the path I'd like to take:

Currently, worker processes are doing the AST parsing and the collection of Elm Lens data from that AST all in one pass. I'm going to split that up into a multi-step process:

1. Parse the AST.
2. Collect just the imports.
3. Determine which other files' exposed top level expressions are required to determine external-/internal-ness for this file's references. a. If other files are required, make a request out to the file system to load those files. b. Parse those files' ASTs. c. Gather up those files' exposed top level expressions.
4. Collect up all other metrics for the base file. External references from global imports should now work correctly, because I can gather all exposed top-level expressions from the ASTs of the imports.

This will require significant modifications to the worker process, so I'm scheduling this for the 0.3.0 release.

My expectation is that users who do not use global imports will experience no change in Elm Lens's performance, as this plan will only incur a few extra checks before proceeding down the typical path. However, it is possible I will discover that users who have many global import statements experience noticeable delays for loading their external references.

Since global imports are not a recommended strategy for long-term/larger projects, I'm going to proceed with this plan, and also form a communication plan which involves both the release notes and updates to the project readme that make the perf impact clear and make a recommendation against using global imports extensively, if at all. It could also help to do some writing about strategies folks can use to better partition modules with very long lists of exports/imports.

[mbuscemi]

This should be fixed in 0.3. @madasebrof, I tested elm-taco-donut on my machine and external references from unqualified imports appear to be counting correctly now. Let me know if you experience something different.