Should suggest explicit *ToExport entries in module manifest

[lzybkr]

Module auto-discovery is fastest when a module has entries for:

• AliasesToExport
• CmdletsToExport
• FunctionsToExport
• Does not use wildcards or $null in these entries.

During module auto-discovery, if any of these entries are missing or $null, PowerShell do some potentially expensive work to analyze the rest of the module (e.g. do reflection on a dll, or parse and analyze the psm1).

If these entries are all present and non null, then this extra work can be avoided and provide a better overall experience.

If one of these entries is missing, PSScriptAnalyzer should suggest adding an entry like:
FunctionsToExport = @()

If one of the entries uses wildcards, PSScriptAnalyzer should suggest replacing the wildcards with an explicit list.

The module type (script or binary) doesn't matter, these entries are needed for the best performance.

[KirkMunro]

Wow, that's a surprise, so +1 for this rule then if that's what's required.

It seems intuitive to me that if you explicitly provide entries for the commands that are exported that you're taking control and saving PowerShell from having to do the guesswork. That's a practice I've been following for a while not.

It doesn't seem intuitive to me that if you don't have a FunctionsToExport attribute in your manifest (because you explicitly removed it) that PowerShell would do extra work to make sure that you did the right thing. That seems like behaviour I'd prefer work differently. If I do use a manifest but I explicitly remove or don't include FunctionsToExport, isn't that clear enough guidance for PowerShell? I guess not, and if that's the case (which Jason is indicating it is), having a rule pick up on this and provide the tip will be helpful.

Now I've got another TODO for my modules, to add back empty command array attributes.

[lzybkr]

Agreed, it's not intuitive.

I have an unreleased change to PowerShell that works like you (and I) expect, but it breaks real world scripts.

The change was made to get better performance, but I'm not keeping the change because of the risk of breaking more scripts, instead I'm hoping we can fix more module manifests to improve performance. This issue is one way I hope to help address the performance problem.

[joeyaiello]

Thanks for filing this here, Jason. I was also not aware that this is how autoloading works until our conversation last week.

I think we'd definitely like to do this one. My only remaining question is whether this should be a warning or an error. Given it's impact, I'd actually make the push for error, but I know it will be extremely noisy given how many people use wildcards today. Our guidance will have to be very clear that this high-impact in order to avoid people just suppressing the output.

[juneb]

Excellent info. I didn't know either. Can someone please update New-ModuleManifest or the TechNet page about module manifests? https://technet.microsoft.com/en-us/library/dd878337(v=vs.85).aspx

It seems that the entries in the module manifest take precedence over Export-ModuleMember statements. Jason, what is the precedence order and is it affected by anything, like module type or nested modules?

[lzybkr]

The module manifest is the final truth if it has a non-null value, but it's not precedence, it's more like filtering. If a psm1 uses Export-ModuleMember, only those names are available to the manifest for export.

I am updating New-ModuleManifest as well.

[juneb]

​Thanks. Appreciate the doc update.

[Sorry to diverge from the rules discussion, but this is really valuable.]

In Joel Bennett's BetterCredentials module, he used Export-ModuleMember to
explicitly export an alias, but because his module manifest had:
FunctionsToExport = @(), the alias wasn't being exported. When I changed
the value of FunctionsToExport to the alias name, it exported.

This is consistent with the non-null value explanation, but it seems that,
in this case, the manifest value took precedence or, perhaps, was evaluated
after, the Export-ModuleManifest command.

So, maybe what we need is an explanation of the order of evaluation.

On Mon, Feb 1, 2016 at 12:49 PM, Jason Shirk notifications@github.com
wrote:

The module manifest is the final truth if it has a non-null value, but
it's not precedence, it's more like filtering. If a psm1 uses
Export-ModuleMember, only those names are available to the manifest for
export.

I am updating New-ModuleManifest as well.

—
Reply to this email directly or view it on GitHub
#434 (comment)
.

Thanks,
June

[KirkMunro]

@juneb: I think you mean AliasesToExport, not FunctionsToExport, since you're talking about aliases, right?

I already knew about the filtering nature of the manifest. It quickly becomes very apparent when you opt for explicit export of functions and cmdlets in your modules and you forget to include a function/cmdlet name in the appropriate manifest attribute.

[lzybkr]

To get the conversation back on track - warning vs error?

I'd lean towards warning. It can be annoying or easy to forget to update your manifest.

We need to encourage frequently used modules to do the right thing (e.g. the Azure module can add 200ms to the command search time (generally a one time cost) because they use wildcards), but smaller modules have much less impact (typically <10ms).

[KirkMunro]

I agree, warning, not error. It's not broken, just recommended for change.

[raghushantha]

"Warning" severity fits here. "Error" is for critical issues.

Jason - Do we enable this rule for modules targeted to any PowerShell version? Or does this behavior affect certain versions?

[lzybkr]

From a performance perspective, the advice only matters from V3 onwards. I think it's safe and reasonable to warn when targeting V2 though.