Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Deconstructor Management for Dynamic Class Instances in CustomJS #79

Merged
merged 5 commits into from
Mar 9, 2024

Conversation

PxaMMaxP
Copy link
Contributor

@PxaMMaxP PxaMMaxP commented Jan 2, 2024

This pull request introduces a new feature for the CustomJS plugin that allows for better management of deconstructors in dynamically loaded JavaScript classes. The changes enable developers to define a 'deconstructor' method in their classes that is invoked before the class instances are discarded, which is particularly useful during the hot reload process.

Main features:

  • An array to store deconstructors along with the associated class names.
  • Asynchronous invocation of deconstructors for performing cleanup operations such as event deregistration.
  • Clearance of the array post-invocation to prevent memory leaks and ensure deconstructors are called only once.

These changes aim to provide developers with enhanced control over the lifecycle of their scripts, avoiding issues like duplicate event listeners that can occur when scripts are re-evaluated during development.

In my specific case, it pertains to an event listener for the file context menu. However, I believe there are multiple applications for this feature.

I have rigorously tested these changes and am confident they will be a valuable addition to the CustomJS plugin. I am eager to receive your feedback and open to suggestions for further improvements.

@PxaMMaxP
Copy link
Contributor Author

PxaMMaxP commented Jan 2, 2024

I have added another setting: 9a97b01

Since I register a context menu event, as explained above, this must also be executed again after the deconstructor due to the reload.

You can now set whether the start-up scripts should be executed again after a reload.

The default is NO. This means that you neither have to use the deconstructor function nor are you hindered by these changes in the basic state. You simply have more options if you want to use them.

@saml-dev
Copy link
Owner

saml-dev commented Jan 6, 2024

Looks great, thanks for contributing. A couple requests:

  1. I merged a PR adding Prettier and it looks like it caused a conflict with your change. Sorry about that, hopefully not too much of a headache to resolve.
  2. Please add documentation for this feature to the README.

@PxaMMaxP
Copy link
Contributor Author

After my pull request, I decided to create my own project for my stuff: I dove in and resurfaced 8,200 lines of code later 🥲

I'll pop in later and adjust the code accordingly and add something to the readme!

Thanks for your patience!

@PxaMMaxP PxaMMaxP force-pushed the feature/deconstructor-management branch 3 times, most recently from 4ca0f40 to e6acac6 Compare January 18, 2024 19:08
@PxaMMaxP PxaMMaxP closed this Jan 18, 2024
@PxaMMaxP PxaMMaxP force-pushed the feature/deconstructor-management branch from e6acac6 to 1604143 Compare January 18, 2024 19:12
Implemented a mechanism to manage deconstructors in dynamically loaded class instances within CustomJS. This includes registering deconstructors upon class evaluation and ensuring proper cleanup before reloading or discarding classes to avoid duplicate event registration. The list of deconstructors is cleared after execution to maintain clean state.
… re-executed when the scripts are reloaded.

Default is NO for compatibility reasons.
Otherwise you suddenly have 500 errors and only the correct line ends are uploaded to git.
@PxaMMaxP PxaMMaxP reopened this Jan 19, 2024
@PxaMMaxP
Copy link
Contributor Author

PxaMMaxP commented Jan 19, 2024

Everything should fit now.

I have taken the liberty of adjusting the treatment of line endings by Prettier: Since only the correct line endings are uploaded to git anyway, it should be up to the local user to decide which ones to use. This made it really difficult for me yesterday to adapt the code accordingly.

If there is anything else you need, just let me know :-)

Edit: I forgot the readme 😅

I will add something later. If that's ok, I'll add a short but concrete example of what you can use it for and how exactly.

…ructor` & re-execute start scripts features.
@PxaMMaxP
Copy link
Contributor Author

So, I hope the addition to the readme is okay 😊

If there is anything else you need, just let me know :-)

@saml-dev
Copy link
Owner

saml-dev commented Mar 9, 2024

README looks good, thanks!

@saml-dev saml-dev merged commit 0b87fdb into saml-dev:master Mar 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants