Stephanevg / PSHTML

Cross platform Powershell module to generate HTML markup language
https://pshtml.readthedocs.io/en/latest/
Other
165 stars 43 forks source link

How to introduce pshtml.document while avoiding a breaking change #293

Open Stephanevg opened 4 years ago

Stephanevg commented 4 years ago

I have wanted to add a main PSHTML object to be able to parse the HTML tree programmatically. The base research and prototype as been done in #218.

Additionally, we have also looked into how to parse existing html static documents using the HTMLAgilityPack and parse it into the PSHTM.Document format in #250

In either case, it involved in quite a heavy refactoring (which would be ok to do if we decide to do it) but the bigger problem resides in the fact that we would break how pshtml currently works.

Problem

This is how pshtml currently works:

div -Class 'plop' -Content {"My Content"}

generates immediatly the following string

<div Class="plop"  >My Content</div>

The current version of pshtml.document prototype:

$e = div -id 'TopheaderDiv' -Class "class1 class2" -Content {

        div "plop scriptblock content in my superdiv" 
} 

$e #output the object
$e.GetChildren()
$e.generatehtml()

GenerateHtml() becomes mandatory to generate the html tree

It might be not that obvious, but the method .Generatehtml()becomes mandatory to actually generate the html structure. If this method is not called, only the objects are sent back.

This would be a breaking change. Learning from the experiences of the recent update of the pester module to version 5 where some breaking changes have been introduced, I want to avoid the current users of pshtml to force them to relearn how to use pshtml.

The main objective would be to create the pshtml.document object - without impacting current existing scripts.

By there, I am not saying we won't increment the major version number of pshtml, but I really want to avoid the unpleasnt user experience of people needing to change:

a) Their existing habits when using pshtml b) Existing scripts that use pshtml.

Proposed Solution / Opening discussions

I have thought this one through, and we have two different ways of how pshtml could potentially work.

I think that we need to have both of these methodologies to work in pshtml to: 1) introduce the pshtml.document which will be the root source of potentially new features / extensions. 2) avoid breaking changes and unpleasant user experience.

Since it is an either / or type of functionality, I think the end user should have the choice of how pshtml should work.

Possible solution 1

Similar to the $VerbosePreference global variable, we could introduce a similar type of variable to control the default way pshtml will behave. (something like $PshtmlOutputPreference perhaps?)

Accepted values:

dynamic
static

The pshtmlOutputPreference variable would define how pshtml behaves:

If set to dynamic pshtml would generate the html tree right away (Just as it is currently working). If set to 'static' pshtml would return the pshtml.document' object. the html tree can than be generated using the.GenerateHtml()` method.

possible solution 2

Similar to the solution 1, we could control the PshtmlOutputpreference via the Pshtml.Configuration.json document.

Possible solution 3

Introduce a new cmdlet that will be the only entry point for the pshtml.document. Something like New-PshtmlObject perhaps?

New-PSHTMLObject would create an object of type pshtml.document. The html structure could only be created via methods located on the pshtml.document' (.AddChild()` for instance).

It will offer a new way of generating the pshtml object structure in a standardized way, without breaking the existing user experience. but this way might be a more clumsy way to use it.

On a technical level, it is still blurry how we could introduce such a dual behaviour without impacting the current user exeperience.

possible solution 4

A combination of solution 1 and 2 :)

I am open to to other possible solution on how to offer this double functionality to the current users of pshtml, while still improving the code base to facilitate the developers work to introduce new features.

Stephanevg commented 4 years ago

Rereading this issue, I feel like the easiest way to to this is to perhaps add changes on the set-htmltag I feel like having a an additional check in Set-htmltag to check for a script variable $PshtmlOutputPreference. Based on that value, we would

  1. Call the implementation just as it is today.
  2. Call the code that would create a new pshtml object, and return object at the end.

We would also need to update the Out-PSHTMLDocument to support passing an object of type pshtml.document

Stephanevg commented 4 years ago

For this to work, we create a new class OutputPreferenceSetting that would inherit from setting. It contain a single property called 'Type' which would be the values of an enum called PshtmlOutputPreference the following values allowed:

dynamic
static 

(Or perhaps 'object' instead of static? - I am Open to suggestions here).

It should have a method called something like .SetPreference() (Or something similar) which would set the value of the Type property to the $pshtmloutputPreference variable.

Stephanevg commented 4 years ago

So, going further in this monologue, I actually enhanced the existing (and year old prototype) to support dynamic and static output preferences (See the following commit on the tagv3 branch)

But the full prototype is working here tagv3Prototype.ps1

It actually works pretty nicely.

We only need to extend the current div function with an additional if statement


    If((Get-pshtmlOutputPreference) -eq 'dynamic'){
        return $tag.getHtml()
    }Else{
        #Is static
        return $tag
    }

When the OutputPreference is 'Dynamic' if will rend the HTML text. When it is set to static, it returns the object. See example below:

Static

Get-pshtmlOutputPreference
$b = div -id "test dynamic" -Class "dynamic" -Content {"Plop"}
$b

#outputs

static

TagName id           Class   Children
------- --           -----   --------
div     test dynamic dynamic {Plop}

dynamic

Same code, this time, $OutputPreference is set to 'dynamic'

In the prototype, the outputpreference test is done by hardcoding the return value of the Get-PshtmlOutPreference function.


Get-pshtmlOutputPreference
$b = div -id "test dynamic" -Class "dynamic" -Content {"Plop"}
$b

#outputs

dynamic
<div id='test dynamic' class='dynamic' >Plop</div>

I have already extended the code in the tagv3 branch branch with the code that creates everything around the $OutputPreference variable (Configuration, function etc...). The next step - if we decide to go further with this - is to try to convert the tag functions with the above if statement, and do some additional tests, and see how the current pester tests will react.