Add graph command for graphing module dependencies
#3635
Conversation
|
I'd actually rather keep this logic internal to the compiler if we possibly can. The idea is that you do pass everything to |
|
Sure, but As another example: if I want to tell webpack/parcel what files need to be watched, I currently have to default to If you're building tooling on top of |
Yes; it should be possible to address both of these issues with something like #2477, so that any given dependency only needs to be built once (across any number of projects), and so that once a dependency has been built, we know it isn't going to change, so we don't need to watch those files.
I agree. Whenever this is a result of the compiler misbehaving that's something I'd like to fix too. See #3145, for instance.
Lots of compilers delegate the responsibility of constructing build plans to third party tools. This can be a big pain, so we've opted for a different approach in PureScript: that build plans should remain internal to the compiler. I think this approach has worked really well so far, and I still believe that the issues you're coming up against can be addressed while sticking to this approach. (I'm open to being persuaded otherwise but I'm not yet convinced.) |
|
Fair points, and I respect that design decision. In which case I definitely
think #3145 and #2477 are worth looking at (probably in that order?) - I
reckon I could have a go at #3145 so I'll try and put a PR together. Thanks
for your help 👊
…On Sun, 12 May 2019, 11:59 Harry Garrood, ***@***.***> wrote:
So it's always gonna compile everything I pass to it at least once, which
isn't great for large projects with lots of dependencies [...] I currently
have to default to "src/**/*.purs" ".psc-package/*/*/*/src/**/*.purs"
Yes; it should be possible to address both of these issues with something
like #2477 <#2477>, so
that any given dependency only needs to be built once (across any number of
projects), and so that once a dependency has been built, we know it isn't
going to change, so we don't need to watch those files.
And in the jenga-tower of frontend tooling I find myself having to rm -rf
output more often then I'd like...
I agree. Whenever this is a result of the compiler misbehaving that's
something I'd like to fix too. See #3145
<#3145>, for instance.
If you're building tooling on top of purs this is a pretty reasonable
requirement though, no? Currently we can only throw a bunch of globs at the
compiler, but it would be nice to have some insight into the build plan.
Lots of compilers delegate the responsibility of constructing build plans
to third party tools. This can be a big pain, so we've opted for a
different approach in PureScript: that build plans should remain internal
to the compiler. I think this approach has worked really well so far, and I
still believe that the issues you're coming up against can be addressed
while sticking to this approach. (I'm open to being persuaded otherwise but
I'm not yet convinced.)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3635 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AC47AG6VZHJONL4EOBSRCWLPU72A7ANCNFSM4HMKAUAA>
.
|
|
Welcome! You might want to check in with @joneshf as he has been looking into using Shake to implement this. |
|
Ooo that sounds fun... 👀
…On Sun, 12 May 2019, 13:07 Harry Garrood, ***@***.***> wrote:
Welcome! You might want to check in with @joneshf
<https://github.com/joneshf> as he has been looking into using Shake to
implement this.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#3635 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AC47AGYISRRBRHOYTYAKW7TPVAB7HANCNFSM4HMKAUAA>
.
|
|
I think this is a great thing to have! What would have to change to get this PR accepted? |
|
Also, I'll throw a proposal for |
For me to get on board I’d want to hear a more persuasive case for it. As I say I think build plans should continue to be the responsibility of the compiler. I think having the compiler delegate the responsibility of calculating what needs rebuilding to external tools has the potential to greatly increase the complexity of build processes; if a number of people create tools which use that information in slightly different ways then we could very easily find ourselves grappling with lots of bugs which are difficult to reproduce and where the location of the problem is difficult to identify (eg “is this a bug in the compiler or in external tooling?”). I don’t think we need to concede this ground to be able to make the compiler smarter and faster in the sense of not doing more work than it needs to in incremental builds. |
|
I don't think the module graph is an internal implementation detail. It's a static artifact of the modules you pass to it, which anyone can derive unambiguously by parsing them. Sure, if it contained information like what needs to be rebuilt, that would be different, but the hierarchy itself is just language semantics. I don't think there's anything wrong exposing it, but I would like a concrete use case where a tool really needs it before we commit to supporting it. |
|
My initial motivation was adding PureScript support to parcel. I could go with the usual glob paths and tell parcel that my |
|
For more motivation, any sort of large scale analysis would re-implement this graph algorithm. In a large codebase, it's very common and tedious to do this analysis by hand. Some examples of what you can do with dependency graphs in the JS ecosystem: Other ecosystems have similar tooling. Are these examples enough to warrant accepting this PR? |
|
Ok, yeah, that's a good point. I was on the fence about the idea of compiling with an entry point; I think I'd prefer to address the underlying issue there, which if I understand correctly is that we want builds to be faster, by coming up with a way of only needing any given dependency at a given version to be built once and then stored somewhere where it's accessible across different projects. Being able to make tooling like the examples you gave would indeed be nice. |
|
I personally would support reopening it. I would probably like a more general analysis of modules though, but it's not necessary for this PR. I would like to see:
|
|
As @hdgarrood noted in purescript/spago#165 (comment), that issue would be another use case for having this command (and would avoid having a dependency on the compiler or part of it) |
|
This is still marked as a draft. We haven't looked at this in a while. Is there anything else to do with this? |
|
If there's agreement that this is something we want then I'll polish it up into a proper PR? |
|
Yes, I think this would be a useful command to have. |
|
I would really really like to see this command in. The reason is that having this command in the compiler would allow us to remove this bunch of code to parse module headers we have in Spago - because you know, it's not good for several reasons: different versions of the compiler parse different things, we should not parse files again if the compiler is going to do it, etc. So having the module graph info exposed by the compiler would allow this and other usecases to have a clean implementation in downstream tools. |
|
@jmackie is there anything I could help with to move this forward? |
|
@f-f lack of time is the blocker 🙈 The code is pretty straightforward but I haven't built it since the original commit. I also haven't looked at this codebase for a while. But if it still builds maybe it's good to go? I'll try and take a look during lunch today. |
|
@natefaubion I'm available to take this PR to a mergeable state, if anything needs to change. Could I ask you to have a review at it and see if we need changes? |
|
@f-f I'm also up for reviewing it. I took a cursory glance and it seems the main thing missing is a few tests. I'll probably also have a couple of nits on code style, but I can fix those myself unless you'd like to. |
|
@kritzcreek I opened #3781 that supersedes this one |
|
Thanks for taking this on @f-f 🙏 sorry I couldn't be more helpful! |
This PR proposes adding a
graphcommand to the compiler, which takes module files as input (similar topurs compile) and returns a JSON description of the module dependency graph. Essentially this just surfaces the output ofLanguage.PureScript.ModuleDependencies.sortModuleswith some serialization logic.I think this will be useful for tooling as it let's us be smarter about what actually needs to be compiled. For example, if I'm writing a bundling tool that takes an entrypoint module I can avoid passing the world to
purs compile(e.g."src/**/*.purs" ".psc-package/*/*/*/src/**/*.purs") by first invoking this command.More concretely, I'd like to improve PureScript support in webpack and parcel, and having this available would really help.
It's very much a first draft, and there are probably a bunch of error cases I need to consider, but comments/feedback would be much appreciated :)