https://discord.com/channels/864614189094928394/872650895744176188/1174406612711125022
thomashoneyman — 11/13/2023 3:24 PM Does anyone know why docs.json files always leave the "reExports" key as an empty array, and it isn't until purs publish is called that we actually go through and associate those re-exports?
natefaubion — 11/13/2023 3:29 PM I'm not sure. It could be at one point re-exports were desugared really early or something.
I guess you could try and construct it eagerly and see what trouble you run into 😄
natefaubion — 11/13/2023 3:58 PM If this is the case, wouldn't that mean current local doc generation with spago would not include re exports?
thomashoneyman — 11/13/2023 4:15 PM I'm not sure what spago does in spago docs, but at a glance running it on my system does include re-exports in the HTML doc.
Looks like it calls purs docs directly https://github.com/purescript/spago/blob/f73cc24edc69d8681b831af1caf9321f8eeb2445/src/Spago/Command/Docs.purs#L44
natefaubion — 11/13/2023 4:18 PM And the purs docs command does all this same work?
thomashoneyman — 11/13/2023 4:23 PM Yea, it does. The command calls docgen: https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/app/Command/Docs.hs#L41
...which calls out to the same collectDocs used by purs publish: https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/app/Command/Docs.hs#L80-L82
Here's that function, for reference https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/src/Language/PureScript/Docs/Collect.hs#L43
It's a short one which does a little setup to ensure we've compiled with docs as an output target, then associates re-exports: https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/src/Language/PureScript/Docs/Collect.hs#L120
f-f — 11/13/2023 4:25 PM The work really happens here: https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/src/Language/PureScript/Docs/Convert/ReExports.hs
What's puzzling me is why we need to do this work post-facto at all
And my current theory is that it's because of the same issue that prevents proper cutoff, which is that re-exports are not first class citizens in a module's interface
But really, the compiler should have all this knowledge during normal compilation, without the need for a second pass
f-f — 11/13/2023 4:33 PM This bit of code mentions "tagging the module with the package it comes from", I wonder if that's why we can't do it earlier (as the compiler doesn't know about packages)
natefaubion — 11/13/2023 4:35 PM I'm not sure why this would matter for doc generation. Are re-exports determined by anything other than the module header?
Does it resolve the canonical location of a re-export?
Regardless, I just don't see why this information wouldn't all be there in the environment. That's why I suspect there's some legacy reason that may not apply anymore.
Re-exports make it all the way to corefn.
So if re-exports in core fn don't require it, I don't see why docs would.
(And this module is ancient, see original commit https://github.com/purescript/purescript/commit/c0aeec91681fb4486051ab607bc8c6f6d40fd924)
natefaubion — 11/13/2023 4:42 PM purescript/purescript#1677 initial issue
thomashoneyman — 11/15/2023 12:52 PM Any advice on reading an externs file? I can decode the cbor to json but it's unreadable. Is there guidance on constructing a json file that more closely matches the data structures in the haskell?
natefaubion — 11/15/2023 12:57 PM Externs are considered internal to the compiler, ie not a public interface (may change at any time). Why specifically do you want to read externs?
thomashoneyman — 11/15/2023 12:57 PM I want to see what the compiler is putting in there wrt re-exports
Specifically, the file appears to have an explicit constructor for re-exports, and from grepping the cbor (decoded to json) it appears that those re-exports are actually being populated
It's not just leaving them empty
But docs generation doesn't use those re-exports for some reason
natefaubion — 11/15/2023 12:58 PM cbor is essentially "binary json" (also known as msgpack). I'm sure there are tools to visualize it out there. But the externs are also all derived afaik from Haskell data types. So anything in externs should be in the Haskell extern data types.
thomashoneyman — 11/15/2023 12:59 PM I do see it in the Haskell extern data types, I just want to make sure they're actually being used
However it does appear they are being used in the file
thomashoneyman — 11/15/2023 1:40 PM Looks like the missing re-exports in docs.json was a specific memory/performance choice, according to: purescript/purescript#3645
The docs.json files specifically exclude re-exports, and they are only collected at purs publish or purs docs time.
natefaubion — 11/15/2023 1:43 PM
this is because computing re-exports requires access to documentation for all of the upstream modules, and we don't have an easy way of getting hold of all of these (which won't adversely affect memory consumption / performance for the regular compilation path)
I don't fully understand this.
But maybe I don't understand docs reexport vs regular reexports or something
thomashoneyman — 11/15/2023 1:46 PM Yea, I'm not sure either. We have access to re-exports via the externs files for upstream modules, and docs generation already requires access to the externs files.
So I'm not sure what is specifically difficult about re-exports
Or hold on, maybe I'm mixing up some stage in the pipeline where externs aren't yet available
natefaubion — 11/15/2023 1:47 PM That is, it's not obvious to me why generating documentation requires upstream documentation. Maybe re-exports weren't in externs before or something, I dunno.
I don't think so. Docs are a codegen target, I would expect externs to always be available.
Or at least, I don't think the memory performance requirements would be different from any other build.
It could also be that there is some design constraint on docs that is no longer really valid. For example, it mentions the "docs library API", and it could be that it was originally designed to be it's own consistent internal format, so some "library" makes assumptions that it's only dealing with docs.json data structures, vs incurring extern dependencies.
natefaubion — 11/15/2023 1:54 PM I personally don't think that docs as a library interface is something that is utilized.
thomashoneyman — 11/15/2023 2:22 PM Looks like the root of the issue is that a "docs module" includes the package name that the re-export comes from, unless it's a module local to the project, in which case it's set to 'null'
"reExports": [
{
"moduleName": {
"package": "purescript-transformers",
"item": ["Control", "Monad", "Error", "Class"]
},(for example)
natefaubion — 11/15/2023 2:23 PM How does it determine that?
thomashoneyman — 11/15/2023 2:24 PM There have historically been other concerns but it looks like they're largely no longer in play (after sleuthing through issues)
thomashoneyman — 11/15/2023 2:24 PM In purs publish at least it's determined from the resolutions file you're required to provide
I'm not sure about purs docs
It's not something the compiler knows about from modules alone
f-f — 11/15/2023 2:30 PM Do we have any info about what this is used for?
Pursuit doesn't need to show the package - it only cares about the module name
thomashoneyman — 11/15/2023 2:31 PM It's at least used to generate the links to modules in other packages
f-f — 11/15/2023 2:31 PM (when displaying the re-export list)
Ah I see, there's an html link that contains the name of the package
Is this a concern of the compiler though?
The compiler doesn't know about packages, and there's no clear path to implementing package support (I think we all agree it would make sense, but there's less agreement on the direction)
So I'd be inclined to say that purs should not worry about it - we give it modules to compile, and they might have re-exports, and it should tell us in the docs what modules are re-exported
thomashoneyman — 11/15/2023 2:36 PM Yea. Feels a bit weird for the compiler to be responsible here.
I understand purs publish and purs docs associating module names to package names to aid in rendering the HTML of the documentation, but the docs.json files seem like they ought not care about the package name association.
(But should still care about re-exports)
f-f — 11/15/2023 2:38 PM The thing is: the generated html docs don't contain references to the package name
This is purely for Pursuit
(I am not sure how it ends up there)
Image
See how this link just points to the module file
In Pursuit it will include the package name in the link
thomashoneyman — 11/15/2023 2:43 PM Interesting. Then it makes even less sense that this is in the docs.json file, because purs publish also sends a moduleMap field mapping module names to package names
(Admittedly, Pursuit ignores that altogether)
f-f — 11/15/2023 2:47 PM This all sounds like the compiler should not worry about any of the package mapping and just stick the re-exported things in the reExports
We can worry about the mapping later. It might help if the re-export module links are flagged with some class so it's possible to find them afterwards with an xpath expression, but it's a small detail
thomashoneyman — 11/15/2023 2:49 PM https://github.com/purescript/pursuit/blob/ea6a220110c379b3553cb107208a5aa40c7c61ba/src/TemplateHelpers.hs#L205-L225
Here's an example of Pursuit relying on this field in docs.json
| D.Module {..} <- pkgModulespkgModules there is the array of docs.json file contents that purs publish sends to pursuit
natefaubion — 11/15/2023 2:52 PM pun globs hissss
thomashoneyman — 11/15/2023 2:52 PM
case inPkg of
D.Local _ -> Nothing
D.FromDep pN _ -> find ((== pN) . fst) pkgResolvedDependenciesthis bit — "Local" vs. "FromDep" — is that package field from the re-exports section of the docs.json I linked before:
"reExports": [
{
"moduleName": {
"package": "purescript-transformers",
"item": ["Control", "Monad", "Error", "Class"]
},in this case we'd have FromDep "purescript-transformers" ...
thomashoneyman — 11/15/2023 2:52 PM (so happy we don't support those)
f-f — 11/15/2023 2:53 PM Yeah, unreadable 😄
thomashoneyman — 11/15/2023 2:53 PM The thing is — we have a map of module names to package names, so if we're going to walk over a bunch of modules to determine what package they came from we can just use that. And if we were to remove this field (package) from the docs.json format, Pursuit could still use the module map that's present in all the backups.
f-f — 11/15/2023 2:54 PM This is what I meant above
The compiler should not deal with packages at all (or do it properly)
thomashoneyman — 11/15/2023 2:56 PM Yea. For compatibility's sake I think it's fine for the compiler to still do the association in purs publish — I don't think Pursuit gets enough info to do the association itself.
But I don't see the docs.json need for it.
natefaubion — 11/15/2023 2:56 PM So what I'm understanding is that
Local doc generation just doesn't care about this at all Purs publish is where module -> package is actually used, and that information is already there in the publish manifest But I guess it "matters" because it gets baked into HTML?
f-f — 11/15/2023 2:56 PM Does it?
thomashoneyman — 11/15/2023 2:57 PM Pursuit is where module -> package is actually used, but that information is included in the publish payload created by purs publish
natefaubion — 11/15/2023 2:57 PM I would assume so, if it's generating links to packages. Does it doctor that up after the fact?
thomashoneyman — 11/15/2023 2:57 PM Otherwise yes that's my understanding as well
f-f — 11/15/2023 2:58 PM Yes, the docs coming out of purs docs don't contain package names
natefaubion — 11/15/2023 2:58 PM I know purs docs doesn't. I mean for pursuit. It's not obvious to me purs docs and purs publish are the same as far as the docs HTML that's sent.
f-f — 11/15/2023 2:58 PM In fact we have to do things in Spago to come up with them after the fact (purescript/spago#1065)
thomashoneyman — 11/15/2023 2:59 PM No HTML is sent
f-f — 11/15/2023 2:59 PM Like, this webpage contains 0 mentions of codec-argonaut, where this module is from
thomashoneyman — 11/15/2023 2:59 PM The contents of the docs.json files are sent to Pursuit in a list at the modules key
natefaubion — 11/15/2023 2:59 PM oh, I thought it pre-baked some of that content.
thomashoneyman — 11/15/2023 2:59 PM And then Pursuit generates the HTML itself
Pursuit reuses docs generation from the compiler as a library but it must be doing some doctoring to how the HTML is generated because it produces different links (including the package names) than purs docs does
Looks like Pursuit specifically reuses HtmlRenderContext, which has hooks for producing links: https://github.com/purescript/purescript/blob/6b49918b9646863e73bbedd1d47f474ba3783408/src/Language/PureScript/Docs/AsHtml.hs#L53-L57
That getDepHtmlRenderContexts I linked before, which looks up the package name, is used to build one of these contexts https://github.com/purescript/pursuit/blob/ea6a220110c379b3553cb107208a5aa40c7c61ba/src/TemplateHelpers.hs#L205-L207
JordanMartinez — 11/15/2023 3:46 PM Here's a summary of what I understand about the code so far.
The type of reExports in the docs.json file is [(InPackage P.ModuleName, [Doc.Declaration])]. Note: that Declaration type is a Doc.Declaration, not AST.Declaration.
On the purs compile -g doc path, we do two things relevant to this issue:
we convert the original AST module to a Doc module and output the result as docs.json. At that point, we have [(P.ModuleName, [AST.Declaration])]. Because we're missing the following information, we set this value to []. a. the 'module-to-package' mapping. b. the corresponding Doc.Declarations for those re-exported AST.Declarations.
we output the desugared & typechecked version of the AST module as an externs file (e.g. externs.cbor), including all of its the exports info.
On the purs publish side, we do the following:
parse the resolutions file to get the 'module-to-package' mapping data.
recompile to ensure the externs files are up-to-date, and ensure docs.json files exist.
read all docs.json files into memory (i.e. docsModules)
For each module (e.g. Module A), we update its docs.json's reExports value by 1) getting the re-exports data from Module A's externs file (i.e. externsReexports), 2) looking up what the corresponding Doc.Declaration is in docsModules based on the declaration identifier found in externsReexports, and 3) store the resulting list in reExports.
JordanMartinez — 11/15/2023 3:51 PM To answer you question, 3.4's 2) is why. Hypothetically, if we stored the docs.json content in the externs file, we would almost no longer have a dependency on the upstream Doc modules. However, since reExports includes the 'module-to-package' mapping data, information we don't have at this point due to lacking the resolutions file, we can't produce a docs.json value.
natefaubion — 11/15/2023 3:56 PM It seems to me then, there are really two formats:
Intermediate docs.json A full rendered docs environment.
And it's using the same data type for both by emptying out re exports.
JordanMartinez — 11/15/2023 3:57 PM Yup! And on top of that, if we fixed that in the compiler somehow, the issue is that older compilers (e.g. v0.14.x) wouldn't have that fix.
So, unless we run purs publish, there's no way to get the reExports, short of writing an CBOR decoder on the externs file, which isn't supposed to be public info in the first place.
natefaubion — 11/15/2023 3:58 PM I mean, that's not totally true. You can always get re-exports by parsing the source file headers.
That is, there's no additional information in externs that doesn't exist in the source regarding exports.
JordanMartinez — 11/15/2023 4:00 PM Right, but my understanding of why this is necessary is because of this issue: purescript/purescript#3503
A nice side effect of doing this would be that it would prevent situations where the docs have incorrect types. For instance, if I write
foo :: String foo = 3
Namely that the externs have been typechecked.
natefaubion — 11/15/2023 4:00 PM I don't follow.
JordanMartinez — 11/15/2023 4:01 PM Though... I guess a docs.json file would indicate things have already been typechecked.
f-f — 11/15/2023 4:01 PM My understanding so far is that purs docs and purs publish use a common code path, and they shouldn't
natefaubion — 11/15/2023 4:01 PM I'm not saying that we shouldn't require the use of externs. I'm just saying, as an external tool, it's possible to construct re export meta data by parsing module headers.
Which can then be used to construct the full docs payload by parsing docs.json files.
JordanMartinez — 11/15/2023 4:02 PM That's correct.
natefaubion — 11/15/2023 4:05 PM Yeah, I think that's correct. The common code path is the shared data type. Basically, the shared data type is the publish type, and the docs type is repurposing that by omitting data.
f-f — 11/15/2023 4:07 PM I think we can just fix it at the root then, without going the route of the external tool - the only usage of purs publish is for Pursuit, and only the latest is used when uploading to pursuit these days
So it's alright if older compilers don't have it
JordanMartinez — 11/15/2023 4:08 PM Is it possible for older compilers to publish packages to the Registry?
Like 0.13.x?
Or is that not supported / won't be supported?
natefaubion — 11/15/2023 4:09 PM To be clear, you are suggesting that we fix this and create a pursuit epoch such that only compilers greater than that version can publish?
JordanMartinez — 11/15/2023 4:09 PM I ask because...
by parsing module headers
In order to support the compilers supported by the Registry (AFAIK, v0.13.x - present), that would mean being able to parse module headers for code outputted by 0.13.x, right? IIRC, language-cst-parser is designed to only parse a single compiler version's source code at a time. I don't think much has changed between them (e.g. kind syntax exports/imports), but that would be one problem to address. right?
The alternative is to not support publishing from older compilers.
f-f — 11/15/2023 4:09 PM It's supported - see all the legacy package import - but no docs are produced in these cases, as they are already on Pursuit
natefaubion — 11/15/2023 4:10 PM I don't think module header syntax has changed recently. I suspect the current parser is very backwards compatible for headers only.
f-f — 11/15/2023 4:12 PM I don't think I'm suggesting that - I'm proposing that:
purs publish continues to do what it does today instead the docs.json start to include info about reExports we change Pursuit to lookup packages from the mapping that is provided when uploading the docs, as Thomas was mentioning above
This should all be backwards compatible
natefaubion — 11/15/2023 4:12 PM The issue is that docs.json can't include the full info of what's in re-exports
docs.json re-exports include the docs of those re-exported items.
Which isn't part of externs.
And which is why it's omitted. That information is part of the full rendered docs environment, which is a thing I made up for the post processing we do.
Like, pursuit could add that information itself based on docs info it already has.
and reexports could just contain the refs, and not docs.
JordanMartinez — 11/15/2023 4:15 PM From the v0.14.0 release notes:
Likewise, kind imports and exports are deprecated and treated the same as a type import or export.
Kind imports are deprecated and will be removed in a future release. Omit the 'kind' keyword instead.
From the v0.15.0 release notes:
Removes deprecated syntax for rows (i.e. #) and kinds (i.e. kind-keyword)
So, it's possible that v0.13.x and v0.14.x compilers will still use the kind KindName export.
f-f — 11/15/2023 4:15 PM Ah I see now, was entirely convinced it was just module names
But no, it's full docs declarations
But still purs codegen -g doc has all of this info??
natefaubion — 11/15/2023 4:16 PM no, it doesn't
it could if it read all the docs.json files and had them in the environment.
f-f — 11/15/2023 4:17 PM So, a second pass
natefaubion — 11/15/2023 4:17 PM That is the memory/performance consideration that was made.
In codegen, we do not include docs of re-exported definitions. And then for purs docs and purs publish a pass is done to load all the docs.json modules and add it.
But to me, it seems like that should all be pushed to pursuit?
Oh I guess not, if you want that for purs docs.
f-f — 11/15/2023 4:21 PM I don't have a specific issue with purs docs - the reason why we ended up questioning all this is because we want to bypass purs publish
In particular this second pass that purs publish does
natefaubion — 11/15/2023 4:21 PM Yeah, I think the assumption I've been making as well is that docs "reexports" is just binding metadata. But it's the full docs.
thomashoneyman — 11/15/2023 4:21 PM I've seen it fail on 0.14.x packages due to a difference in RowList syntax or something like that. I don't think it's just parsing the export list in the partial parse.
f-f — 11/15/2023 4:22 PM I think we'd be fine if we taught Pursuit how to deal with the vanilla docs that are generated by purs docs rather than having a custom generation
(that relies on the reExport being populated by purs publish - as purs docs also implements this)
monoidmusician — 11/15/2023 4:23 PM I think you would need dependencies to also be published in that scenario?
f-f — 11/15/2023 4:23 PM What do you mean?
monoidmusician — 11/15/2023 4:24 PM like where are you pulling the re exports from? if they aren't embedded, they would have to already exist on pursuit?
f-f — 11/15/2023 4:25 PM purs docs will fill in the Re-exports section when it generates the html
Literally calling the same code that is called in purs publish as well
f-f — 11/15/2023 4:25 PM See here ^
I guess part of why Pursuit needs the package disambiguation is that it needs to do the association that you are talking about
natefaubion — 11/15/2023 4:27 PM So, there's two separate things:
There's the re-export docs There's the re-export package name
The latter is solvable by data that already exists, but you still need to do the first thing in some tool if you want to replace purs publish. purs docs only fills in the data in memory, it doesn't persist it to disk in anyway.
f-f — 11/15/2023 4:28 PM purs docs will generate files on disk
natefaubion — 11/15/2023 4:28 PM I understand that, but it's not the files used by pursuit.
f-f — 11/15/2023 4:28 PM Right, because Pursuit has its own custom pipeline
natefaubion — 11/15/2023 4:29 PM You are suggesting that "publish" should just send the baked HTML generated by purs docs?
f-f — 11/15/2023 4:29 PM We can still send the docs.json and have pursuit generate the html
Just not in a custom way
natefaubion — 11/15/2023 4:30 PM I guess I'm lost then. What has changed?
I think the main sticking point is how you get re-export docs.
Not re-export package names.
f-f — 11/15/2023 4:31 PM Earlier you had to fill the reExport list, but if pursuit uses the vanilla purs docs then it's not needed
natefaubion — 11/15/2023 4:31 PM No that's not true.
f-f — 11/15/2023 4:31 PM I am not following
f-f — 11/15/2023 4:31 PM See here
The re-exported definitions are all there
natefaubion — 11/15/2023 4:32 PM the re-export list contains the docs of the items you are re-exporting. You still have to fill that information in somehow. What does purs docs have to do with that?
f-f — 11/15/2023 4:32 PM It fills the list in the same way as purs publish
With collectDocs
natefaubion — 11/15/2023 4:33 PM Can you describe the workflow you are suggesting for a third party tool implementing publish?
f-f — 11/15/2023 4:34 PM The issue is not the workflow, the issue is that we'd be changing Pursuit and that's work
natefaubion — 11/15/2023 4:34 PM I think we are on separate pages, and I'm not sure where that's happening.
This is my understanding:
The discussion was about replacing purs publish with a third-party workflow Re-exports are a sticking point, because they aren't included in codegen docs.json. Re-exports include potentially package information and the docs of those re-exports. We don't include the docs information for re-exports because that would require having that information during compilation. That would mean a) reading all docs.json files b) re-generating documentation metadata based on information in the typechecker environment. For purs publish or purs docs, the compiler reads all docs.json files and adds this information in-memory, but does not persist it back to disk. If we change the compiler to include this information somehow so that the third party tool can use it, it effectively creates a docs epoch for that version of the compiler. If we don't change the compiler to include this information, the third-party publish needs to reassemble re-export metadata on its own, by reading all docs.json files, and by inferring re-exports from source files (as there is no other publicly available source for this information)
natefaubion — 11/15/2023 4:43 PM For those particular points, I'm not sure where your suggestion of using purs docs or changing pursuit fits in.
f-f — 11/15/2023 4:43 PM My observation is that Pursuit is the only user of the JSON files produced by purs publish.
So we can either keep the format as is and go around it with a new tool, or change the format and change the only consumer
I guess a third option would be to add a second pass to purs codegen -g to fill the reExports in back to disk
But that's more involved I guess
natefaubion — 11/15/2023 4:47 PM
We don't include the docs information for re-exports because that would require having that information during compilation. That would mean a) reading all docs.json files b) re-generating documentation metadata based on information in the typechecker environment.
It's not obvious to me that b) is significant overhead.
Yes, it does redundant work. But that work does not seem to be expensive work (to me).
That is of course me assuming that the comments are still in the externs, and I don't know if that's true.
They just aren't in doc.json format.
f-f — 11/15/2023 4:49 PM If we don't want to burden the compilation pipeline with this we could get purs docs to fill this data back into the docs.json
Not sure how kosher that is
natefaubion — 11/15/2023 5:01 PM It seems to me that any changes will require some sort of docs epoch somewhere:
- If we change the format, only the compiler that produces that format can publish it, and only the pursuit that consumes it can accept it.
- If we fill in the format, only the compiler that fills it in can publish with the third-party tool.
- If we assemble re-export data manually from source files, only headers supported by the third-party parser can be consumed (assuming this is 0.15.x).
My personal opinion is that the last option is the least disruptive. I do not believe users publish using significantly older compiler versions (though I think 0.15.7 vs 0.15.11 is likely, for example). One can also always write/fork a custom header parser that supports older syntax.
One can parse externs sure, but these can change from point release to point release. If this parsing is re-implemented in a third party tool, this is very likely to get out of sync. There would have to be a commitment by the tool to track compiler releases almost immediately to assert that things can still publish, adding backwards compatibility where necessary. To me, this seems untenable.