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

Repeated code between server previewGeneration.js and client EmbedFrame.jsx #2805

Closed
8 tasks
lindapaiste opened this issue Dec 29, 2023 · 0 comments · Fixed by #3384
Closed
8 tasks

Repeated code between server previewGeneration.js and client EmbedFrame.jsx #2805

lindapaiste opened this issue Dec 29, 2023 · 0 comments · Fixed by #3384

Comments

@lindapaiste
Copy link
Collaborator

Increasing Access

Our codebase becomes more maintainable if functions are defined in one place only.

Feature enhancement details

I noticed that there is a function resolvePathsForElementsWithAttribute which is identical in both files. Other parts of the code are similar but different.

Tasks:

  • Look at the Git history for each file and read any discussions on the PRs and issues that led to changes.
  • Reuse functions from the server code in the client component instead of recreating it.
  • Add explanatory comments in the code.
  • Add unit tests.
  • Make improvements to the code itself, for example using RegExp test() to check for truthiness rather than match() !== null.

Questions:

  • Which parts of the code are the same and can be reused?
  • For the parts which differ, is the difference due to necessity of the client vs server environment?
  • Do we even need the server version at all? This code was previously used for the /embed/ endpoints, but those are disabled right now due to phishing concerns. I'm not sure what it would take for us to re-enable it. It might make sense to delete it. See Bug/phishing #1967, Restore Present view that doesn't allow phishing #1971, etc. for background.
@lindapaiste lindapaiste self-assigned this Dec 29, 2023
@raclim raclim added this to the MINOR Release for 2.13.0 milestone Mar 8, 2024
@raclim raclim removed this from the MINOR Release for 2.13.0 milestone Mar 29, 2024
Harshit-7373 added a commit to Harshit-7373/p5.js-web-editor that referenced this issue Mar 5, 2025
Harshit-7373 added a commit to Harshit-7373/p5.js-web-editor that referenced this issue Mar 10, 2025
raclim added a commit that referenced this issue Mar 13, 2025
…---#2805

Removed code duplication between 2 files. #2805
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment