search

PowerShell / Polaris

A cross-platform, minimalist web framework for PowerShell
https://powershell.github.io/Polaris/
MIT License
511 stars 114 forks source link

RFC #002: Bundle PSHTML with Polaris #165

Open TylerLeonhardt opened 5 years ago

TylerLeonhardt commented 5 years ago

RFC #002

An unexpected usecase for Polaris has popped up recently. More so than the main use case (APIs) - Small websites and self-service portals.

These are almost always powered by @Stephanevg 's PSHTML.

This RFC is to add PSHTML to the RequiredModules section of the Polaris psd1 so that users automatically have PSHTML when they install Polaris from the PowerShell Gallery.

The module is only a few megabytes so it's not a huge burden if the user doesn't need it.

This doesn't break anything... But it's an added dependency so it needs to be called out.

Cc @ChendrayanV

RFC Checklist (You can remove this or leave it at the bottom of the RFC)

See RFC 001 for a sample.

brettmillerb commented 5 years ago

If it's not the only way to do something then should it really be a dependency, breaking or not?

bgelens commented 5 years ago

Not a big fan of bundling modules if there is no real dependency. What is next? Suggestion, keep polaris as a bare core module and publish a "superset" module containing both (and in the future possibly more) as required module and call it polarisplus / polarissdk or something

ChendrayanV commented 5 years ago

Good point! Thanks for your insights!

gaelcolas commented 5 years ago

Please don't bundle it in, the dependency model is the way forward... What if someone wants to use PSHTML with another server, does he still have to install Polaris?

As @bgelens pointed out, there's no dependency, it's just a convenience thing (for some), and that's why it should be the other way around:

a 'small website' powered by Polaris & PSHTML should have them as dependency

If one module changes at a higher cadence than another, separating them reduces the scope of the changes and makes both easier to maintain over time (lower working memory).

Tiberriver256 commented 5 years ago

I think this needs a bit of clarification on bundle vs dependency. Adding PSHTML to the RequiredModules section of the Polaris module does not mean that the module would be downloaded during a build and published to the PowerShell gallery as part of the Polaris module.

The PowerShell dependency management is the same as Node or NuGet. We can specify a minimum version, maximum version, both or none and if there is a module already on the PC matching those requirements it will be left alone otherwise Polaris would automatically install the MaxVersion or latest version (if max not specified) from the PowerShell gallery for convenience.

Hopefully that clears up some of your concerns @gaelcolas.

I'd be interested in talking more about:

  1. Is the convenience worth it?
  2. How introducing view engines could be done in a way that: a. No one will ever notice if they don't care b. Doesn't add overhead

Question #1 - Is the convenience worth it?

We would have to implement a generic function to support view engines that might look something like this:

$PolarisViewEngineScriptBlock = {
    param($FilePath, $Options)

    Import-Module PSHTML
    . $FilePath
}

$ViewEngine = New-PolarisViewEngine -ScriptBlock $PolarisViewEngineScriptBlock -FileExtension ".pshtml"

Set-PolarisViews -Directory './views' -Engine $ViewEngine

Then we could reference it via a route like:

New-PolarisGetRoute -path '/' -ScriptBlock {

   $Options = @{
    FirstName = "Bob"
    LastName = "Dylan"
   }

   $Response.Render('index', $Options)
}

If we placed a file called index.pshtml in the ./views folder with the following content:

$FirstName = $Options.FirstName
$LastName = $Options.LastName

html {
    head {
        title "PSHTML + Polaris Demo"
    }
    body {
        h1 ("Hello " + $Firstname + " " + $Lastname)
    }
}

We would get:

<html>
   <head>
      <title>PSHTML + Polaris Demo</title>
   </head>
   <Body>
      <h1>Hello Micah Rairdon</h1>
   </Body>
</html>

Basic view engine functionality... Now talking about baking in PSHTML or another view engine we would be talking about getting rid of this:

$PolarisViewEngineScriptBlock = {
    param($FilePath, $Options)

    Import-Module PSHTML
    . $FilePath
}

$ViewEngine = New-PolarisViewEngine -ScriptBlock $PolarisViewEngineScriptBlock -FileExtension ".pshtml"

Set-PolarisViews -Directory './views' -Engine $ViewEngine

and replacing it with this:

Set-PolarisViews -Directory './views' -Engine 'PSHTML'

How much work is that saving people? Probably not a whole lot but it does mean you can do some pretty cool demos right off the bat and support other great work out there. If we do go down this route I would definitely still advocate we expose the API to allow easy definition of custom view engines and we leave the mimimumVersion and maximumVersion wide open so people can run whatever version they want.

Question #2 - Can we do it in a low-impact no-maintenance mode?

Probably not "no-maintenance" but it's a pretty low chance we would effect anyone's work if we implemented as suggested above. The only thing I would expect us to encounter is probably having to do some initial triage work on issues that should have been put in the PSHTML repository.

If we think it's low enough maintenance, isn't going to impact Polaris users, helps build the PS community and gives us enough convenience to do it. I would say we could throw it in there. View engines are handy handy. Another good one I looked at at one point is EPS cool stuff!

jeremymcgee73 commented 5 years ago

I'm not a huge fan of adding dependencies that aren't really needed. I just feel like setting the precedent is not something that we want. I realize PSHTML is just a few MB. It isn't that hard to install PSHTML, if it is needed. I think view engines/middleware, etc should be a module that has a dependency on Polaris, if it's uniquely to be used with Polaris. Not the other way around.

I could see a use case, running Polaris in a container, where you want your image to be very very small. I just don't think forcing PSHTML does much good, it's not "minimal".

brettmillerb commented 5 years ago

Take poshbot for instance, that has plugins which are available but not required or dependent with similar low footprint but if someone isn't going to use the giphy one then they're not going to want it installed.

TylerLeonhardt commented 5 years ago

Thank you all for your feedback!

What if we included the generic View code that @Tiberriver256 mentioned, along with the PSHTML specific helper script:

$PolarisViewEngineScriptBlock = {
    param($FilePath, $Options)

    Import-Module PSHTML
    . $FilePath
}

$ViewEngine = New-PolarisViewEngine -ScriptBlock $PolarisViewEngineScriptBlock -FileExtension ".pshtml"

Set-PolarisViews -Directory './views' -Engine $ViewEngine

But do NOT bundle PSHTML? We can simply handle if PSHTML is not available and prompt the user to Install it.

Is that a decent compromise?

TylerLeonhardt commented 5 years ago

This RFC process really works :)

Jaykul commented 5 years ago

A metamodule called PolarisHtml sounds like the right thing to me ...

It's worth pointing out that if you add a dependence on (or even a script that recommends people install) another module, you're forever on the hook to keep track of it. Is it still good? Was it abandoned? Has that module been hijacked a-la event-stream :trollface:

Stephanevg commented 5 years ago

To me, adding it as a dependency like @Tiberriver256 is suggesting sounds like a good approach. Prompting the user at installation as @TylerLeonhardt suggests is also a good compromise I think.

I would add, that having the possiblity to install it in a later step using a cmdlet like 'Install-PolarisAdditionalModules' (or something in that line) could be interesting for the end user in case he decided not to install it, and in a later step, changed his mind. This could then also potentially be extended in the futur for other modules that would make sense to have with Polaris.

I actually disagree with renaming the existing PSHTML module to something new like 'PolarisHTML' what @Jaykul is suggesting. I think It will confuse the users more then help them, as in the end, it will still be PSHTML, but simply renamed. It will missguide users and as they will probably miss communications about updates etc.. concerning PSHTML, thinking they don't have it place. It will also add that extra step maintenance into the Polaris module. For each new version of PSHTML, the maintainers of the Polaris Module will need to create a new version of PolarisHTML. Which I see as an extra unnecessary step.

Jaykul commented 5 years ago

You're missing the point @Stephanevg I strongly oppose adding the dependency on an HTML module to this module -- or vice-versa.

Polaris does not Require an HTML rendering module at all.

Likewise, PSHtml does not Require a web server module.

I don't want to rename anything. I'm just saying: if people want to install them together, a new, third module called "PolarisPSHTML" (or something like that) could be created that is a meta-module (i.e. nothing but a manifest requiring the other two). That's the right way to "bundle" modules that don't require each other.

Frankly -- this isn't a tool for newbs anyway, let people install their own modules. If you want to promote particular modules over others, have a wiki or about_* docs page listing related modules... don't add fake dependencies or install scripts.

Tiberriver256 commented 5 years ago

@Jaykul - Is there a good example out there of how a metamodule or plumbing to support metamodules like that would work?

Would it be something like Yeoman generators where it searches the installed modules for ones starting with the keyword Polaris and expects them all to have a common exported function like Get-ViewEngine? or am I over complicating it and you're just thinking a module that adds required modules for both and extends or overwrites the Polaris Set-PolarisViews function or something similar?

Jaykul commented 5 years ago

@Tiberriver256 The AzureRM module. Oh, wait, you said good example. Hmm ... 😜

The AzureRM module is a weird one because they CHEAT to work around an old bug in PowerShell: their manifest has no dependencies -- instead, they import the modules in the .psm1 and generate the nuspec for their Module package by hand (so PowerShellGet sees the dependencies, with versions, but PowerShell doesn't).

Anyway -- yes, you're over-complicating it. I just meant a module manifest that says:

RequiredModules = @('Polaris','PSHTML')

That would be the perfect place to put the override for the views, too.