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

Rewrite router to NOT use CoffeeScript #6041

Merged
merged 2 commits into from
Nov 1, 2021
Merged

Conversation

shaunanoordin
Copy link
Member

@shaunanoordin shaunanoordin commented Oct 29, 2021

PR Overview

CoffeeScript is bad. We want basic JavaScript. This PR refactors the PFE router to not use the bad CoffeeScript.

  • Only one file is replaced: app/router.cjsx is now app/router.jsx

Testing

This affects a foundational aspect of PFE, so please double check everything!

Testing URL: https://pr-6041.pfe-preview.zooniverse.org

Expected behaviour:

Status

Ready for review. Required for #6037

Note that I didn't optimise or future-proof anything. If you want to, go ahead and make an in-line suggestion and I'll commit them to the branch. 👌

@shaunanoordin shaunanoordin requested review from eatyourgreens, mcbouslog and a team October 29, 2021 15:39
app/router.jsx Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Oct 29, 2021

Coverage Status

Coverage remained the same at 50.094% when pulling ca93a2e on decaffeinate-router into 07dbf18 on master.

@eatyourgreens
Copy link
Contributor

As of #6035 and #6039 FEM project links and About page links don't use react-router to redirect to NextJS, so that makes testing a bit simpler, I suppose.

@eatyourgreens
Copy link
Contributor

Nothing is broken in projects, Talk or the project builder, after some very quick, initial clicking of links on the staging site.

<Route path="organization-status/:owner/:name" component={OrganizationStatus} />
</Route>

<Route path="todo" component={() => <div className="content-container"><i className="fa fa-cogs"></i> TODO</div>} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, seriously, what is this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

</Route>

<Route path="todo" component={() => <div className="content-container"><i className="fa fa-cogs"></i> TODO</div>} />
<Route path="dev/workflow-tasks-editor" component={require('./components/workflow-tasks-editor')} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-Authored-By: Jim O'Donnell <[email protected]>
@shaunanoordin
Copy link
Member Author

PR Update

  • media.exports = code changed to export const routes =
  • main.cjsx's import line for the router updated accordingly.

Thanks, Jim!

@shaunanoordin shaunanoordin merged commit 2c8bc70 into master Nov 1, 2021
@shaunanoordin shaunanoordin deleted the decaffeinate-router branch June 24, 2024 23:01
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.

3 participants