Pylons / deform

A Python HTML form library.
Other
416 stars 160 forks source link

Chicken-and-egg problem between repos of Deform and deformdemo #472

Closed stevepiercy closed 3 years ago

stevepiercy commented 3 years ago

Would it solve the chicken and egg problem, making it possible for us to test simultaneous changes in both Deform and deformdemo in GitHub Actions or Travis, without specifying from where to pull the changes?

Not by itself, containers just make running the tests more easily reproducible across different systems. The core problem I think is that deform and deformdemo are by there nature tightly coupled, so their releases, PR's, features etc should be completely in sync. The best way I know how to do that is a monorepo; i.e. merging deformdemo into deform. I would understand if that is not an option, but I think that is fundamentally the easiest solution.

But to keep the current split, a few options crossed my mind:

  1. create a "next" branch on both repositories that is used in the tests. The next branch is either equal to master, or has some extra's bolted on to progress to the next step. In the deform repo a PR to master is tested using the next branch of deformdemo. A PR for deformdemo@master is tested using deform@next. So a new feature (that influences both repo's) requires PR's to be made to both next branches. These PR's are merged without testing, and then new PR's are made for merging next into master. This all feels quite cumbersome, especially when iterations are needed to get stuff ready. Also this doesn't allow multiple PR's for different features to co-exist. Effectively what I would want is that when I make a PR into Pylons/deform@master from tdamsma/deform@, the tests are run using tdamsma/deformdemo@<some-new-feature and not Pylons/deformdemo@master. Probably technically possible to set up, but it sounds way to convoluted

  2. use git submodules and tests against whatever the current commit of the submodule is. I am not so experienced with this so not really sure if this would actually work

  3. require a certain order for changes: first commit/merge a PR in deformdemo, and only the make a PR for deform. Use some kind of decorator to skip tests for unavailable features depending on the deform version, something like pytest.mark.skipif(deform.__version__ <= "3.11")

To be honest I don't like any of these options much. Perhaps this should be a separate discussion for the containerization?

Originally posted by @tdamsma in https://github.com/Pylons/deform/issues/471#issuecomment-685113416

stevepiercy commented 3 years ago
  1. That is cumbersome. We would need to have special configurations for tox.ini, .travis.yml, and maybe elsewhere, making sure they are properly aligned. It solves one problem while creating another.
  2. We used to have submodules in Pylons Project repos to build docs using the custom Pylons Sphinx theme. We almost always screwed it up because of forgetfulness or updating submodules correctly. We stopped doing that, and instead created a new repo as a Python package that gets released to PyPI.
  3. We currently follow this practice. It's not without problems, but it at least gets us over the hump.
tdamsma commented 3 years ago

Assuming the repo's are split because of a separation of concerns, I am thinking what are the actual concerns (correct me if I'm wrong):

So the issue is with the source code management, both libraries depend on each other, So how about merging the source code to one (deform would be the logical choice) but keep the separate repo's around for issue tracking and documentation.

stevepiercy commented 3 years ago

First of all, thanks for kicking me in the butt to rethink what we're doing and why.

The demo was part of Deform at one point long ago, but it was split out for reasons that are lost to the sands of time, and before I started using Python. I've got my feelers out to hopefully get some insight.

These days I'm not concerned about keeping the demo separate, except to archive it, if it were to be brought back into Deform. I would lock down the deformdemo repo, and move all its issues and PRs to Deform (this is easy in GitHub now). Personally I think it is unnecessary overhead that just gets in the way of being a contributor. I've never liked this separation and it does not make sense to me.

Let's assume we bring the demo back into Deform. Here are my thoughts.

  1. Reorganize the code. I'd want to emulate Pyramid, but add the demo. This allows only Deform's source to be easily released (currently it includes its tests, which is a sloppy practice).
    • demo/
    • docs/
    • src/deform/
    • tests/
  2. Include only Deform's source code in the release to PyPI, using the spiffy code reorganization.
  3. Adjust Tox, Travis, and other(?) configuration.
  4. Adjust how we deploy https://deformdemo.pylonsproject.org/ (this is a matter for the Pylons Project infrastructure team to discuss)
  5. Profit!
tdamsma commented 3 years ago

I think merging would be a great improvement. If you reformat your thoughts as a checklist we can get started ;) As part of step one, perhaps deformdemo@master can be rebased to a branch of deform so the commit history is preserved too. I might give it a try, my git-fu could use a challenge

sydoluciani commented 3 years ago

Sorry guys, I will not contribute in merging Deformdemo to Deform since I think we should separate them. you are welcome if you want to do so, but consider the time you are going to spend against time being saved with new strategy. currently there is only couple of pull requests per month that is managable.

The only reason original designer of Deform removed Deformdemo, was to prevent bloating Deform, but included functional test from Deformdemo to make sure any changes to Deform will pass functional tests.

The future of Deform is being simple, and yet independent to any of the javascript, even bootstrap, once we include npm/yarn package manager, and move all javascripts out of Deform, at some point we might move to flexbox instead of bootstrap.

The easiest way to over come the inter dependency of Deform and Deformdemo is to remove functional tests from Deform tox.ini, moving all extra widgets to Deformdemo, but knowing that any changes in Deform, shall pass the unit test and as well as functional test in Deformdemo.

stevepiercy commented 3 years ago

I agree that Deform's repo would be bloated. However Deform the released package would not be bloated by merging. @sydoluciani is there harm from bloating the repo that I overlooked?

@tdamsma I omitted many details of what a merge would require. It is more complex than my abbreviated overview. I am in no rush to merge the repos. There are many other issues in Deform that have a higher priority over this one, and we have a workaround but it is not documented.

The suggestion to remove the functional and functional3 environments in tox from Deform sounds like a quick and easy option, but I want to keep them in for two situations.

  1. We continue to get assurance that changes to Deform alone do not cause unexpected UX failures.
  2. If we know that there is a paired PR for the functional tests in deformdemo, then we can ignore their failure in Deform.

Our contributing.md is hand-wavy in this regard. I will make a PR to improve that section. Here's an attempt to clarify by inserting a new section and a header. I'd appreciate both of your feedback.


Functional tests

...

When to add or edit functional tests

If you add a feature to Deform, or change a feature in Deform that causes any functional test to fail, then you also must submit a pull request that includes sufficient functional tests to the deformdemo repository paired to your pull request in Deform. Before submitting any pull request, all tests must pass using tox in your local development environment.

All pull requests automatically run tests. However when there are paired pull requests in both Deform and deformdemo, we have a chicken-and-egg situation. Each repository cannot pull the version of the other referenced in the pull request to run functional tests. Automated testing will fail for both pull requests.

We work around this as follows.

  1. Ensure that all of Deform's automated builds pass, except for the functional test build, for that pull request.
  2. Merge the pull request.
  3. Now deformdemo can pull the desired version of Deform. Restart the failing jobs for deformdemo to ensure that functional tests pass.
  4. When all automated builds tests pass, then merge the pull request in deformdemo.

Running functional tests

To run functional tests.

...

stevepiercy commented 3 years ago

See PR #474. Review and comments appreciated over there.

Out of all this discussion, I might still reorganize the repo for best practices, but leave out the demo, so that I can include only Deform core and not its tests when releasing to PyPI.

sydoluciani commented 3 years ago

I agree that Deform's repo would be bloated. However Deform the released package would not be bloated by merging. @sydoluciani is there harm from bloating the repo that I overlooked?

There is no harm if you @stevepiercy and @tdamsma have time to make required changes and maintain it in future, but it definitely increases the maintenance hours in long run, and limits our maneuver ability to implement new strategies in future.

Deform facilitates overriding templates or introducing new widgets, but that does not mean for every new widget or new template need to have test. contributors can submit their new widgets or templates to custom_widgets of Deformdemo even without test, and move it to Deformdemo official templates if test is written for them.

@tdamsma I omitted many details of what a merge would require. It is more complex than my abbreviated overview. I am in no rush to merge the repos. There are many other issues in Deform that have a higher priority over this one, and we have a workaround but it is not documented.

@stevepiercy I totally agree with this part. right now , tests are passed against Firefox but failing against Chrome or Opera just because of the way functional tests are written and needs to be fixed. the way Selenium handles click and moving around with arrowup and arrowdown is handled differently in Firefox and Chrome, and some functional tests should be rewritten to use Selection instead of click and arrow. then there are bugs or issues that are not addressed for a while, and on top of that we need to improve or fix issues that is raised after Bootstrap upgrade. with all these I don't think we gain much with merging Deform and Deformdemo, since there are only few pull requests per month.

The suggestion to remove the functional and functional3 environments in tox from Deform sounds like a quick and easy option, but I want to keep them in for two situations.

  1. We continue to get assurance that changes to Deform alone do not cause unexpected UX failures.
  2. If we know that there is a paired PR for the functional tests in deformdemo, then we can ignore their failure in Deform.

Our contributing.md is hand-wavy in this regard. I will make a PR to improve that section. Here's an attempt to clarify by inserting a new section and a header. I'd appreciate both of your feedback.

Functional tests

...

When to add or edit functional tests

If you add a feature to Deform, or change a feature in Deform that causes any functional test to fail, then you also must submit a pull request that includes sufficient functional tests to the deformdemo repository paired to your pull request in Deform. Before submitting any pull request, all tests must pass using tox in your local development environment.

All pull requests automatically run tests. However when there are paired pull requests in both Deform and deformdemo, we have a chicken-and-egg situation. Each repository cannot pull the version of the other referenced in the pull request to run functional tests. Automated testing will fail for both pull requests.

We work around this as follows.

  1. Ensure that all of Deform's automated builds pass, except for the functional test build, for that pull request.
  2. Merge the pull request.
  3. Now deformdemo can pull the desired version of Deform. Restart the failing jobs for deformdemo to ensure that functional tests pass.
  4. When all automated builds tests pass, then merge the pull request in deformdemo.

Running functional tests

To run functional tests.

...

tdamsma commented 3 years ago

then there are bugs or issues that are not addressed for a while, and on top of that we need to improve or fix issues that is raised after Bootstrap upgrade. with all these I don't think we gain much with merging Deform and Deformdemo, since there are only few pull requests per month.

Just want to give my outside perspective on this:

I would not be surprised if the

Now I get the sentiment that functional frontend tests should be a this, but in practice, in my experience at least, I have not come across a web project with functional tests that "just work" as expected. Also with Cypress (which makes strings claims about not being flaky, haha) I have experienced great difficulty to run tests consistently. They work (from a docker container) on one machine, and then not in CI (using same docker container) etc. I wasted a few hours getting the simplest widget I wanted to contribute to be properly tested, and then @stevepiercy wasted some more. In that time I could have contributed a lot more valuable stuff to deform. If it were up to me I would:

Ultimately tests are there to help move a project forward, not to hold it back

stevepiercy commented 3 years ago

combined together form a barrier for entry for now contributors, and might explain the lack of.

I agree in part. There are more reasons that Deform does not have a lot of contributors. More than any other reason, many contributors have moved on to JavaScript frameworks, such as React. Deform has not had a committed maintainer for many years. There has been a lack of follow up on open issues and PRs for about six years. After I finished triaging issues last month, I found that only a couple original contributors gave their time to help resolve the issues they brought up.

Before the setup was cumbersome, it was unworkable. The setup was broken for almost two years until @sydoluciani (mostly) and I fixed and documented it, and I became a maintainer just this summer. We're now taking the next step, which is being worked on in either PRs. @sydoluciani is taking that detailed documented process and learned experience, and containerizing it, giving developers an easier option to get started and run.

I acknowledge that there are problems. Collectively we collaborate to fix them, according to priorities and available time.

then @stevepiercy wasted some more

I learned that drag and drop with Selenium does not always work. I also followed the path you took to ultimately choose the Select2Sortable plugin, and gained a greater understanding of how others found different solutions that worked for them, some of which found happiness in jQueryUI's draggable option. I would not call that a waste of time.

limiting the tests to use Webtest, no more selenium

We need something that can test manipulation of the DOM, such as a Sequence (of Mappings) with Ordering Enabled. I do not know if Webtest can do that. If it can replace Selenium while maintaining 100% functional test coverage, I would accept pull requests to change. I am open to other options than Selenium, too.

Ultimately tests are there to help move a project forward, not to hold it back

Tests are there to preserve the quality of a project and prevent it from regressing. Perhaps I misunderstand your statement, but are you saying that we should change the Pylons Project official project policy of 100% test coverage?

sydoluciani commented 3 years ago

then there are bugs or issues that are not addressed for a while, and on top of that we need to improve or fix issues that is raised after Bootstrap upgrade. with all these I don't think we gain much with merging Deform and Deformdemo, since there are only few pull requests per month.

Just want to give my outside perspective on this:

I would not be surprised if the

Selenium will exist even after the merge, therfore contribution instruction still will be the same.

  • requirement for functional (selenium) tests (which are notorious to write, often flaky, difficult to debug etc
  • impossibility to have the tests run on CI idue to chicken and egg problem (so I cannot verify for myself that what my contribution is valid before I need to involve maintainers)

Agree with your point, and not every contributor wants to be involved with Selenium or any kind of testing, not because it is not a good thing, but because it is cumbersome and need to spend time, but again merging won't help, since merge means we are merging selenium tests from Deformdemo to Deform. you could just submit a pull request to Deformdemo for your custom widgets, place it in custom_template with no test. if contributors can just submit their custom widgets with no test in Deformdemo, we have way more widgets that we have now. what you did with demonstrating your widgets in Deformdemo was great, it is showing how contributors can introduce new resource in resource registery and creating new widgets and we can use it as an example and base for new contributions.

combined together form a barrier for entry for now contributors, and might explain the lack of . So maybe the lack of PR's makes streamlining this less of priority, but just maybe all these hurdles combined have put off a few people that were willing to contribute

Now I get the sentiment that functional frontend tests should be a this, but in practice, in my experience at least, I have not come across a web project with functional tests that "just work" as expected. Also with Cypress (which makes strings claims about not being flaky, haha) I have experienced great difficulty to run tests consistently. They work (from a docker container) on one machine, and then not in CI (using same docker container) etc. I wasted a few hours getting the simplest widget I wanted to contribute to be properly tested, and then @stevepiercy wasted some more. In that time I could have contributed a lot more valuable stuff to deform. If it were up to me I would:

  • limiting the tests to use Webtest, no more selenium

    • assure a widget renders (no server errors)
    • js dependencies are correctly loaded
    • correct data is passed to client
    • server correclty parses client responses And then use deformdemo quickly be able to manually verify in the browser of choice that the some js edge case works or doesn't

This is another change on top of merging that needs its own implementation time and effort, and even if we have time for both, not advisable to do both at the same time. but now that I am thinking, you have a good point, Deform only responsible to get the data and produce sequence or map of field objects and unit test with minimum web test shall suffice.

if you have time, why not write a webtest completely for Deform, and make Deformdemo a separate entity with structure that people can submit their widgets and templates easily with no test if it stays in custom_widget and if they write test, we move it to official template folder of Deformdemo ? please let us know what framework or language you were going to choose for webtest if you had the time ?

Ultimately tests are there to help move a project forward, not to hold it back

sydoluciani commented 3 years ago

then there are bugs or issues that are not addressed for a while, and on top of that we need to improve or fix issues that is raised after Bootstrap upgrade. with all these I don't think we gain much with merging Deform and Deformdemo, since there are only few pull requests per month.

Just want to give my outside perspective on this: I would not be surprised if the

Selenium will exist even after the merge, therfore contribution instruction still will be the same.

  • requirement for functional (selenium) tests (which are notorious to write, often flaky, difficult to debug etc
  • impossibility to have the tests run on CI idue to chicken and egg problem (so I cannot verify for myself that what my contribution is valid before I need to involve maintainers)

Agree with your point, and not every contributor wants to be involved with Selenium or any kind of testing, not because it is not a good thing, but because it is cumbersome and need to spend time, but again merging won't help, since merge means we are merging selenium tests from Deformdemo to Deform. you could just submit a pull request to Deformdemo for your custom widgets, place it in custom_template with no test. if contributors can just submit their custom widgets with no test in Deformdemo,

When I say no test, I mean minimum test or at least working in Deformdemo. as long as other contributors can use these custom widgets to have a faster start in their projects or in contributing back to Deformdemo

we have way more widgets that we have now. what you did with demonstrating your widgets in Deformdemo was great, it is showing how contributors can introduce new resource in resource registery and creating new widgets and we can use it as an example and base for new contributions.

combined together form a barrier for entry for now contributors, and might explain the lack of . So maybe the lack of PR's makes streamlining this less of priority, but just maybe all these hurdles combined have put off a few people that were willing to contribute Now I get the sentiment that functional frontend tests should be a this, but in practice, in my experience at least, I have not come across a web project with functional tests that "just work" as expected. Also with Cypress (which makes strings claims about not being flaky, haha) I have experienced great difficulty to run tests consistently. They work (from a docker container) on one machine, and then not in CI (using same docker container) etc. I wasted a few hours getting the simplest widget I wanted to contribute to be properly tested, and then @stevepiercy wasted some more. In that time I could have contributed a lot more valuable stuff to deform. If it were up to me I would:

  • limiting the tests to use Webtest, no more selenium

    • assure a widget renders (no server errors)
    • js dependencies are correctly loaded
    • correct data is passed to client
    • server correclty parses client responses And then use deformdemo quickly be able to manually verify in the browser of choice that the some js edge case works or doesn't

This is another change on top of merging that needs its own implementation time and effort, and even if we have time for both, not advisable to do both at the same time. but now that I am thinking, you have a good point, Deform only responsible to get the data and produce sequence or map of field objects and unit test with minimum web test shall suffice.

if you have time, why not write a webtest completely for Deform, and make Deformdemo a separate entity with structure that people can submit their widgets and templates easily with no test if it stays in custom_widget and if they write test, we move it to official template folder of Deformdemo ? please let us know what framework or language you were going to choose for webtest if you had the time ?

Ultimately tests are there to help move a project forward, not to hold it back

tdamsma commented 3 years ago

Let me start off by explicitly expressing my gratitude to @stevepiercy and @sydoluciani for reviving deform.

I would not call that a waste of time.

@stevepiercy I admire your optimism! Indeed I also learned a bit in the process.

Perhaps I misunderstand your statement, but are you saying that we should change the Pylons Project official project policy of 100% test coverage?

I was not advocating for lowering test coverage, but to rethink the scope of what needs to be tested (and leave the functional/selenium part out). I think @sydoluciani agrees:

Deform only responsible to get the data and produce sequence or map of field objects and unit test with minimum web test shall suffice.

To wrap things up, I will work on adding my custom widgets to deformdemo in the propose structure. When @stevepiercy and @sydoluciani agree on a way forward, I will gladly chime in again and see what I can contribute to Deform.

sydoluciani commented 3 years ago

Let me start off by explicitly expressing my gratitude to @stevepiercy and @sydoluciani for reviving deform.

I would not call that a waste of time.

@stevepiercy I admire your optimism! Indeed I also learned a bit in the process.

Perhaps I misunderstand your statement, but are you saying that we should change the Pylons Project official project policy of 100% test coverage?

I was not advocating for lowering test coverage, but to rethink the scope of what needs to be tested (and leave the functional/selenium part out). I think @sydoluciani agrees:

I think for new wedgets if they placed in custom_templates that represents un-official Deformdemo widgets, there should be no test requirement to bring in more contributores to load their custom widgets and templates. as long as it is functional in Deformdemo. if other users started using these new widgets or templates, they will create issues indicating there is a need for better support or coverage, then we can work on new feature or test coverage and move them to official directory within Deformdemo.

At the end of the day, @stevepiercy is making the last call since he is the product maintainer.

Deform only responsible to get the data and produce sequence or map of field objects and unit test with minimum web test shall suffice.

To wrap things up, I will work on adding my custom widgets to deformdemo in the propose structure. When @stevepiercy and @sydoluciani agree on a way forward, I will gladly chime in again and see what I can contribute to Deform.

Thank you @tdamsma , your custom widgets can be a base for new contributors. These are perfect example of how Deform allows overriding widgets, templates and resources. Thank you.

stevepiercy commented 3 years ago

The single existing custom widget was implemented without much thought toward expansion. The following are my thoughts about how to improve the situation for expansion with more custom widgets. Here's the current structure:

Going forward, I would like to see.

tdamsma commented 3 years ago

Other configuration that I overlooked or erred upon? I think you forgot about the resource_registry which is incidentally also the bit I struggle most with.

  • You need to define it separately from the widget, however it is tightly coupled.
  • It needs to be passed to the Form render function, possibly conflicting if you have multiple (say one per custom widget)

So the process for custom resources (jss, css) is now

And all this for what actually? I don't understand the design, it seems to allow for decoupling between resource and widget. But they are by nature quite tightly coupled. Perhaps if there could be a parallel registry that uses BS4 instead of BS3? But then that would also require a different template, so basically you would need to change the entire widget altogether.

If at all possible, it would make life a lot easier if the css and js resources could be defined on the Widget class, just like the template. Basically replacing the requirements attribute with js_resources and css_resources.

stevepiercy commented 3 years ago

Yeah, I was a little too hand-wavy about static resources with this one, so I just now edited it.

  • Adjust popup method as needed for new paths.

But I think you can still work with it. In the widget docs, does the deform.Field.set_default_resource_registry() class method help? I think that allows you to reuse the existing default_resource_registry and populate it with additional resources within your custom widget.

tdamsma commented 3 years ago

So that allows overriding the default registry. So when we add multiple custom widgets to deformdemo they will have to share the same resource registry. Supporting js_resources and css_resources could be a non breaking deform change right? Why is the resource registry implemented the way it is, am I overlooking something or is it to put it bluntly a useless, overengineerd abstraction? I can't think of a usecase where it would come in handy

found this package that uses it: https://github.com/robinharms/deform_autoneed. Don't really know what it does and seems pretty abandoned

stevepiercy commented 3 years ago

I don't quite follow you. Is your frustration with the resource registry itself or how the resources are rendered in the page?

I would have to dig into history and git comments to understand why the resource registry was designed this way. I never gave it much thought, assuming that the original authors had good reasons to do so.

I agree that the rendering is sub-optimal. I think https://github.com/fanstatic/js.deform solves the rendering issue, and Fanstatic resolves a few other issues.

stevepiercy commented 3 years ago

Here are some commits that mention resource. There was even an attempt back in 2013 to autogenerate assets with bower (RIP).

tdamsma commented 3 years ago

My point is that there is a layer of abstraction (the resource_registry) that forces you to split things that logically belong together (paths to resources and widgets that require them), and that makes combining widgets from different sources difficult (because you you then have multiple registries, which isn't supported). So this abstraction has a cost, but it has (as far as I can see) no benefit.

stevepiercy commented 3 years ago

I don't have an answer for you. Sorry. You'll have to ask the original authors or co-conspirators at that time.

I can insert a key/value into the resource_registry, so I don't see the difficulty you describe.

I'm trying to understand your beef, but not doing well. Maybe I don't understand your definition of "combining widgets". Do you mean having multiple widgets in a form or page, grabbing a jQuery plugin and all of its dependencies to create a widget, or something else?

tdamsma commented 3 years ago

ideally I would like to have all the code for an example widget in a single folder/module. Then in an application i do from .my_custom_widget import SuperWidget, put it in a schema and render it. And thin if I want to add another widget from somewhere else in the same form, I can do that. However with the current architecture that isn/t possible, as I would have to import and combine two resource_registries and pass them to deform too. These registries can't "self register" in deform at import time, no they have to be passed to deform.Form at render time.

sydoluciani commented 3 years ago

I agree with all the points @tdamsma is making, but these will go away if we move toward webpack/npm/yarn.

Deform originally started in 2010, just after node.js first release in 2009 , and at that time, original author wanted to make it easy for end user to load proper css or javascript for each widget with just calling get_widget_resources() on widget, and then load it in template as it explained in form tutoiral, search for reqts in document:

@property
    def reqts(self):
        return self.wiki_form.get_widget_resources()
   <tal:block tal:repeat="reqt view.reqts['css']">
        <link rel="stylesheet" type="text/css"
              href="${request.static_url(reqt)}"/>
<tal:block tal:repeat="reqt view.reqts['js']">
        <script src="${request.static_url(reqt)}"
                type="text/javascript"></script>
    </tal:block>

Without having resource registery, end user should have guessed the proper javascript or css file name based on widget name, and looking in to static directory and try and error.

The same functionality will be provided, using Webpack.

Webpack will transpile all javascript and css files in one file, then copies it in static directory, and we include that one file in our widget resource or in our page.

Probably we should talk about resource registery when we have decided to move to Webpack or other tools that provide some sort of registry or automation.

sydoluciani commented 3 years ago

Other configuration that I overlooked or erred upon? I think you forgot about the resource_registry which is incidentally also the bit I struggle most with.

  • You need to define it separately from the widget, however it is tightly coupled.
  • It needs to be passed to the Form render function, possibly conflicting if you have multiple (say one per custom widget)

So the process for custom resources (jss, css) is now

  • Define the requirements in the Widget class as a tuple of tuples using some sort of unique identifier for your widget, e.g. jsoneditor requirements: Tuple[Tuple[str, None], ...] = (("jsoneditor", None),)
  • Create a new resource registry
  • Translate that particular key to a set of js and css resources
    resource_registry.set_js_resources(
      "jsoneditor", None, "deformdemo:static/jsoneditor/jsoneditor.min.js"
    )
    resource_registry.set_css_resources(
      "jsoneditor",
      None,
     "deformdemo:static/jsoneditor/jsoneditor.min.css",
    )
  • import the custom resource registry so it can be passed to the form render
      form = deform.Form(
          schema,
          buttons=("submit",),
          resource_registry=my_custom_resource_registry_that_has_resources_for_all_used_custom_widgets,
      )

And all this for what actually? I don't understand the design, it seems to allow for decoupling between resource and widget. But they are by nature quite tightly coupled. Perhaps if there could be a parallel registry that uses BS4 instead of BS3?

I am not seeing any Bootstrap css or Bootstrap javascript in either requirements, or registery !!!! can you please point out the line in source code that brings Bootstrap in to Widget ?

From what I understood, Bootstrap classes are statically injected in templates, and not loaded through widget resource registery, and that is the reason Bootstrap files are included statically and not through resource registary:

<link rel="stylesheet"
          href="${request.static_url('deform:static/css/bootstrap.min.css')}"
          type="text/css" media="screen" charset="utf-8"/>
<script src="${request.static_url('deform:static/scripts/jquery-2.0.3.min.js')}"
            type="text/javascript"></script>
    <script src="${request.static_url('deform:static/scripts/bootstrap.min.js')}"
            type="text/javascript"></script>

But then that would also require a different template, so basically you would need to change the entire widget altogether.

If at all possible, it would make life a lot easier if the css and js resources could be defined on the Widget class, just like the template. Basically replacing the requirements attribute with js_resources and css_resources.

sydoluciani commented 3 years ago

I agree it is a bit confusing, but if we move all those widgets from widget.py to Deformdemo, other than Widget class, SequenceWidget class and MappingWidget class, Deform itself has no dependency to any of the CSS or Javascripts.

We upgraded Bootstrap with no problem if excluding failed tests left from previous works, and any changes we made so far to fix or improve was in tests and functional tests. I only changed 3 lines of code in Deform so far.

sydoluciani commented 3 years ago

ideally I would like to have all the code for an example widget in a single folder/module. Then in an application i do from .my_custom_widget import SuperWidget, put it in a schema and render it. And thin if I want to add another widget from somewhere else in the same form, I can do that. However with the current architecture that isn/t possible, as I would have to import and combine two resource_registries and pass them to deform too. These registries can't "self register" in deform at import time, no they have to be passed to deform.Form at render time.

It needs a redesign for sure, but we should consider using Webpack or other frontend tools to provide the resources to Deform. I would leave it until we have the proper frontend tool ready and discussed.

tdamsma commented 3 years ago

It needs a redesign for sure, but we should consider using Webpack or other frontend tools to provide the resources to Deform. I would leave it until we have the proper frontend tool ready and discussed.

Unless you want to bundle all js and css for all widgets into a single file I don't see what the build system has to do with this. A better build system could help generate up to date minimal js/css per widget built from source instead of vendored in code via copy/paste. But still every widget would have to somehow define which js/css files need to be loaded to make it work.

stevepiercy commented 3 years ago

@tdamsma I think I understand the problem you describe now. Thanks for explaining. I don't know of a good way forward, but I'm open to suggestions for a plan forward and a proof of concept.

@sydoluciani indeed, Bootstrap is not registered in Deform, but only included in its package. In my apps, I use Jinja2 with Bootstrap requirements specified as static assets in the <head>, and inject the rendered form into my templates. deformdemo does something similar in deformdemo/templates/main.pt.

Whoever solves the "JavaScript frontend dependency integration with Python schema declaration backend" problem will be wealthy beyond their wildest dreams. Perhaps Fanstatic or deform_autoneed has already solved this issue? I could reach out to both maintainers and ask them some questions. If you have some questions that you'd like to ask, I can collect them and be liaison. I can also preface my questions with the current state of Deform and options of where we would like it to move toward.

  1. What was the motivation behind your package?
  2. What problems does your package solve that Deform does not solve?
  3. What criticisms, if any, do you have of Deform?
  4. What other thoughts would you like to share about the state of server-side rendering libraries?
sydoluciani commented 3 years ago

@tdamsma I think I understand the problem you describe now. Thanks for explaining. I don't know of a good way forward, but I'm open to suggestions for a plan forward and a proof of concept.

@sydoluciani indeed, Bootstrap is not registered in Deform, but only included in its package. In my apps, I use Jinja2 with Bootstrap requirements specified as static assets in the <head>, and inject the rendered form into my templates. deformdemo does something similar in deformdemo/templates/main.pt.

Whoever solves the "JavaScript frontend dependency integration with Python schema declaration backend" problem will be wealthy beyond their wildest dreams. Perhaps Fanstatic or deform_autoneed has already solved this issue? I could reach out to both maintainers and ask them some questions. If you have some questions that you'd like to ask, I can collect them and be liaison. I can also preface my questions with the current state of Deform and options of where we would like it to move toward.

  1. What was the motivation behind your package?
  2. What problems does your package solve that Deform does not solve?
  3. What criticisms, if any, do you have of Deform?
  4. What other thoughts would you like to share about the state of server-side rendering libraries?

Fantastic uses intermediary Python library like js.bootstrap to load the Bootstrap JavaScript and CSS, and I think it is redundant, however my question to Fantastic developers is, if they have thought of writing a Python wrapper around Webpack instead to produce front end skelton for Python web frameworks. like being able to download, and produce output by instantiating a Python class or calling a Python function instead of running npm and webpack manually.

I was going to say we don't need any kind of translatore that found this package It would be interesting if we could use it to load the javascript modules instead of hard coding them in source code.

Looked up for Python Webpack Wrapper and found this package and then this package but they both make the solution more complex than it is now.

stevepiercy commented 3 years ago

Discussion opened in https://github.com/fanstatic/js.deform/issues/24

stevepiercy commented 3 years ago

ideally I would like to have all the code for an example widget in a single folder/module. Then in an application i do from .my_custom_widget import SuperWidget, put it in a schema and render it. And thin if I want to add another widget from somewhere else in the same form, I can do that.

@tdamsma @sydoluciani put some work into a structure that we think is a pretty good compromise in https://github.com/Pylons/deformdemo/pull/92 which is now on the deformdemo master branch (I will backport it to the 2.0-branch later).

sydoluciani commented 3 years ago

I liked what @tdamsma suggested for having separate module for each Deformdemo form, like the directory structure @stevepiercy suggested, instead of having all Deformdemo forms in __init__.py, however changing the architecture of official Deformemo takes a considerable amount of time, and at the moment we can't afford, but we can start with un-official version. after all we would like to demonstrate how easy widgets and templates can be overriden, and having separate module for each Deformdemo would help.

@tdamsma can either include new Deformdemo forms and widgets with replacing Text Input Widget, or re-architect within un-official Deformdemo diretory structure and include new Deformdemo forms. not a bad idea to have an alternative example for Deformdemo for future reference.

stevepiercy commented 3 years ago

This issue has wandered off course, so I'll attempt to round it all up.

Deform and DeformDemo 2.0.15 were both released last week, and there were a ton of improvements.

Effectively we cannot solve the original issue while there are two separate repos. That's OK. To mitigate the issue and other points brought up during the discussion, note the following.

  1. We now have Unofficial Deform Demo where contributors may add custom widgets that are not supported with the usual rigorous tests and documentation that is required by the Pylons Project.
  2. We are now using GitHub Actions (GHA) instead of Travis-CI and Appveyor. All jobs in a PR now pass or fail within 6 or 7 minutes total, instead of the previous 30 minutes or more.
  3. We have two active branches per project, master and 2.0-branch. The setup.py across both projects and versions now specifies its version under development with .dev0 appended. This makes it possible to install only PyPI released versions or allow development versions to be installed. Furthermore, the script in Deform, run-selenium-tests.bash, checks out the correct version of deformdemo from GitHub. Conversely deformdemo checks out the correct version of Deform as specified when GHA runs tox, which in turn installs requirements-dev.txt.
  4. Documentation for contributing in Deform is now rock solid.
  5. For each branch, when adding or editing functional tests, the process is:
    • Create a new branch from master for both projects, make changes, and run tox successfully.
    • Only when tox is successful locally, then commit, push, and create PRs against master.
    • Maintainer check out the branches and run tox successfully to verify tests pass.
    • Maintainer merges PR to deformdemo first. This allows Deform to use the latest version of deformdemo for its functional tests in GitHub Actions. It is OK to do this merge because we do not release deformdemo to PyPI and its purposes are only for development, testing, and demonstration.
    • When GHA passes in Deform, merge the PR to master.
    • Repeat for 2.0-branch, backporting the changes from master.
  6. Deform Demo is currently automatically deployed using Ansible, and keeping it in a separate repository makes sense.
  7. Containerization of Deform and DeformDemo is now available.

@tdamsma it would be awesome if you were to contribute your Select2Sortable widget to the Unofficial Deform Demo.

I think that covers it all.