Skip to content

JS: recognize library inputs when the library exports "through" a function#7137

Merged
erik-krogh merged 1 commit into
github:mainfrom
erik-krogh:functionExport
Nov 17, 2021
Merged

JS: recognize library inputs when the library exports "through" a function#7137
erik-krogh merged 1 commit into
github:mainfrom
erik-krogh:functionExport

Conversation

@erik-krogh
Copy link
Copy Markdown
Contributor

@erik-krogh erik-krogh commented Nov 15, 2021

@erik-krogh erik-krogh added the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Nov 15, 2021
@github-actions github-actions Bot added the JS label Nov 15, 2021
@erik-krogh erik-krogh added no-change-note-required This PR does not need a change note and removed Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish labels Nov 16, 2021
@erik-krogh erik-krogh marked this pull request as ready for review November 16, 2021 09:58
@erik-krogh erik-krogh requested a review from a team as a code owner November 16, 2021 09:58
result = unique( | | call.getCalleeNode().getAFunctionValue()).getAReturn()
)
or
// the exported value is a function that returns another import.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if this pattern is common in this syntactic form? If so, it would probably be worth it to support this kind of reexport directly in our import graph as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we can put this kind of "reexport" directly into the import graph.

At the import site it requires a user to write something like:

var lib = require("lib")(conf);

So it's only "reexported" when the client calls the function.

Copy link
Copy Markdown
Contributor

@esbena esbena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with a suggestion for another PR.

@erik-krogh erik-krogh merged commit 474c808 into github:main Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JS no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants