Skip to content

Comments

Abstract Snowboard plugin constructors and destructors#561

Merged
bennothommo merged 3 commits intodevelopfrom
fix/snowboard-construct
May 19, 2022
Merged

Abstract Snowboard plugin constructors and destructors#561
bennothommo merged 3 commits intodevelopfrom
fix/snowboard-construct

Conversation

@bennothommo
Copy link
Member

@bennothommo bennothommo commented May 18, 2022

Previously, Snowboard plugins had been written to append the main JavaScript constructor. While this worked, it prevented the ability to destruct the plugin during construction (ie. prevent double initialisation on plugins, for example), as the "detach" method which is fired on destruction is added after construction.

This change abstracts the constructor and destructor to separate methods run after the true construction of the JavaScript object is complete. A bonus is that the constructor no longer needs to include "snowboard" as one of the parameters - this instead is done on the true construction.

This is technically a breaking change, as parameters are not passed to the true constructor anymore, but instead passed to the new construct method (no longer valid as of d1f740c). People who have written Snowboard plugins will need to replace constructor() with construct(), and destructor() with destruct() in their plugins. The construct() method no longer needs Snowboard as the first parameter, so this can be dropped.

In addition, if they call the super() method within their constructor, they will need to change this to super.construct() instead.

Previously, Snowboard plugins had been written to append the main JavaScript constructor. While this worked, it prevented the ability to destruct the plugin during construction (ie. prevent double initialisation on plugins, for example), as the "detach" method which is fired on destruction is added *after* construction.

This change abstracts the constructor and destructor to separate methods run after the true construction of the JavaScript object is complete. A bonus is that the constructor no longer needs to include "snowboard" as one of the parameters - this instead is done on the true construction.
@bennothommo bennothommo added maintenance PRs that fix bugs, are translation changes or make only minor changes needs review Issues/PRs that require a review from a maintainer labels May 18, 2022
@bennothommo bennothommo added this to the v1.1.9 milestone May 18, 2022
This should make the Snowboard constructor changes backwards-compatible.
@bennothommo bennothommo added Status: Completed and removed needs review Issues/PRs that require a review from a maintainer labels May 19, 2022
@bennothommo bennothommo merged commit ec29f96 into develop May 19, 2022
@bennothommo bennothommo deleted the fix/snowboard-construct branch May 19, 2022 01:23
LukeTowers added a commit that referenced this pull request Jul 15, 2022
* develop: (65 commits)
  Update maximum supported PHP version for 1.1 (#603)
  Update EditorConfig link (#602)
  Apply consistent path normalisation to fix Windows test
  Fix view maker unit test for Windows
  Outputs time respecting backend preferences (#572)
  Rebuild Snowboard agian
  Revert "Rebuild Snowboard"
  Rebuild Snowboard
  Allow a string selector for the form in a request
  Use correct line breaks for Windows tests
  Backport ViewMaker tests from 1.2 branch
  farsi spelling correction (#579)
  Add Winter 1.2 as version option in bug report
  Adjust early termination of requests from success/error
  Increase stale check to 6 months
  Let composer resolve current php process in scripts (#563)
  Fix track input not keeping timeout
  Add jQuery AJAX prefilter as early as possible
  Add additional debugging for errors thrown in events
  Abstract Snowboard plugin constructors and destructors (#561)
  ...

# Conflicts:
#	composer.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant