Exports tagging #2969
Exports tagging #2969
Conversation
🦋 Changeset detectedLatest commit: 1b8f08c The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
* main: Added lint error Better documentation examples Minor fix to examples ordering Fixed Fn examples for FnDocumentation Removed interactiveExamples Refactoring examples to use custom makeFnExample fn Continued small docs improvements Small tag doc fixes Generic doc cleanup
| @@ -225,6 +225,15 @@ example2 = {|x| x + 1}`, | |||
| }), | |||
| ], | |||
| }), | |||
| maker.make({ | |||
There was a problem hiding this comment.
We also could just keep this out of the public interface for now, if we want.
|
(I addressed the main comments above) |
There was a problem hiding this comment.
- Happy to see "Exports" and "Imports" panels in viewer
- tbh I'm still not convinced that tagging each export is necessary, but we'll see, can remove it later as we discussed
- extra stuff showing in "Exports" seems critical, not sure why that happens
- I'll take a look at versioned-components issues
| return [key, _value]; | ||
| }); | ||
| const exports = bindings.filter( | ||
| (value, key) => value.tags?.exportData !== undefined |
There was a problem hiding this comment.
Somehow this leads to tagged unexported values showing in "Exports":
There was a problem hiding this comment.
(Fixed! The problem is that it should have been .exportsData(), not .exportData.)
| let _value = value; | ||
| if (exportNames.has(key)) { | ||
| _value = value.mergeTags({ | ||
| exportData: { sourceId: this.sourceId, path: [key] }, |
There was a problem hiding this comment.
This is more convoluted than I'd like, I see why you had to do it this way, but now I want to move this logic deeper in the reducer, store exports and bindings directly in context.
(I'll do it later)
| ? internalOutputR.value.externals.explicitImports | ||
| : internalOutputR.value[field]; | ||
| const dict = new SqDict( | ||
| field === "exports" |
There was a problem hiding this comment.
These ternaries are complicated, I'd suggest this:
const bindings = makeTopDict(internalOutputR.value.bindings);
const exports = makeTopDict(internalOutputR.value.exports.mergeTags({ name: sourceId }));
const imports = makeTopDict(internalOutputR.value.externals.explicitImports);With makeTopDict containing the rest of the common code.
| valueAst: astR.value, | ||
| valueAstIsPrecise: true, | ||
| path: new SqValuePath({ | ||
| root: "bindings", |
There was a problem hiding this comment.
Shouldn't this vary for different fields?
…quiggle into Exports-tagging * 'Exports-tagging' of github.com:quantified-uncertainty/squiggle: fix hub build
This does a few things:
exportDatadata tag.SqProject.index.VDictvalueToString()method. (Adds a space between commas. This required changing several tests).Closes #2924