Skip to content

Fix for Build Breaking When Deployed to Release #2507

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

Merged
merged 10 commits into from
Oct 18, 2023
Merged

Conversation

raclim
Copy link
Collaborator

@raclim raclim commented Oct 16, 2023

A majority of these changes are temporary solutions to address current issues with deploying a production build to Release.

Changes:

  • Reinstalls @pmmmwh/react-refresh-webpack-plugin as a dependency rather than a devDependency to temporarily address error where Release cannot find this package. (The root cause appears to be an issue where Babel and Webpack appear to interpret the build as different environments i.e development vs production. Some possible solutions were found but I had some difficulty implementing them. Since this temporary solution doesn't seem too severe and to get the next release out soon, I'm thinking this could probably be addressed in a separate issue?)

  • Also bumps up the above package a patch release number (0.5.10 -> 0.5.11).

  • Removes lines 14 & 97 (which dispatches the action stopSketch()) from client/modules/IDE/pages/IDEView.jsx to address an error where the target origin does not match the recipient for the iFrame.

Changes that have been updated with @lindapaiste's suggestions commented below:

  • Comments out lines related to isOverlayVisible and setIsOverlayVisible in client/modules/IDE/pages/IDEView.jsx to address an issue where the PreviewFrame was not receiving any mouse/keyboard interactions due to setting display: block on line 196. I wasn't sure what the exact vision was for here (I'm sorry if it's already noted somewhere!) but I decided to temporarily comment this out for now!

  • Removes lines 1 & 52 from client/routes.jsx to address an error where the component property for Route.proptypes cannot be set (this also appears exclusively in Release for me).

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@release-com
Copy link

release-com bot commented Oct 16, 2023

Release Environments

This Environment is provided by Release, learn more!
To see the status of the Environment click on Environment Status below.

🔧Environment Status : https://app.release.com/public/Processing%20Foundation/env-07eca90074

@lindapaiste
Copy link
Collaborator

  • Comments out lines related to isOverlayVisible and setIsOverlayVisible in client/modules/IDE/pages/IDEView.jsx to address an issue where the PreviewFrame was not receiving any mouse/keyboard interactions due to setting display: block on line 196. I wasn't sure what the exact vision was for here (I'm sorry if it's already noted somewhere!) but I decided to temporarily comment this out for now!

The intention of the overlay stuff was to prevent mouse events from firing on the sketch while dragging the resize panel. The mistake here (which I should have caught!) is that the initial state should be false instead of true. In the previous class-component version we were directly manipulating HTML to do this.

onChange={() => {
this.overlay.style.display = 'block';
}}
onDragFinished={() => {
this.overlay.style.display = 'none';
}}

I think if you change the initial state to false then you should be able to add those lines back in.

There's probably a much cleaner way to implement the intended behavior using pointer-events: none in the CSS, rather than displaying an invisible overlay.

@lindapaiste
Copy link
Collaborator

  • Removes lines 1 & 52 from client/routes.jsx to address an error where the component property for Route.proptypes cannot be set (this also appears exclusively in Release for me).

I don't recall why I wrote this as two separate assignments. I wonder if it works this way?

Route.propTypes = { 
  ...RouterRoute.propTypes,
  component: PropTypes.elementType.isRequired
};

instead of

Route.propTypes = { ...RouterRoute.propTypes };
Route.propTypes.component = PropTypes.elementType.isRequired;

@raclim
Copy link
Collaborator Author

raclim commented Oct 18, 2023

Sounds good, thanks so much for the suggestions! I'll try those out and if all looks good, will hopefully be able to deploy before the end of the day!

@raclim raclim marked this pull request as ready for review October 18, 2023 17:05
@raclim raclim merged commit 55de42d into develop Oct 18, 2023
@raclim raclim deleted the fix/deduce-error branch October 18, 2023 20:59
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