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

draft: CRA migration, DX slim-down #92

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

draft: CRA migration, DX slim-down #92

wants to merge 5 commits into from

Conversation

mattxwang
Copy link
Member

@mattxwang mattxwang commented Aug 6, 2021

This draft PR (in its current state):

  • installs create-react-app via react-scripts
    • sneaky fixes include changing the source URL for module resolution, and the homepage field in package.json
  • changes the API_URL to read from the DEV env var; defaults to https://members.uclaacm.com/app if we're not in dev, which currently causes a CORS issue if not deployed to it
  • replaces node-sass with sass
  • removes everything related to:
    • Jenkins
    • nginx
    • babel
    • husky
    • lint-staged
    • jest and enzyme
    • the fetch polyfill
    • webpack
  • fixes newfound linter issues (unused imports, failed nan checks, etc.)
  • switches the body font from Lato to Open Sans

Locally, it builds and runs; we only run into problems once we hit the backend, as this introduces CORS issues.

To merge this, I think we need to:

  • somehow resolve this CORS issue and/or figure out how to deploy this
    • if we keep the rest of our deploy setup, then we need to resolve CORS, the final dockerized bundle, API subdomains, etc.
    • idea: maybe make a staging backend on heroku!
  • re-add the relevant linting tools (though I'm fine with just eslint, as that's not a regression)
  • add a CI action to ensure that yarn build passes
  • documents these changes somehow

@mattxwang mattxwang marked this pull request as draft August 6, 2021 19:03
@mattxwang
Copy link
Member Author

@reginawang99

@reginawang3495
Copy link

Just to confirm, was the intention of the PR to move completely away from Docker?

And what could I help with here? I tested on my computer and yarn install and yarn start opens up the app properly.

@mattxwang
Copy link
Member Author

Just to confirm, was the intention of the PR to move completely away from Docker?

And what could I help with here? I tested on my computer and yarn install and yarn start opens up the app properly.

Partially, yes - let me write this down, didn't get the opportunity to do it before. I'll tag you in the issue!

@mattxwang
Copy link
Member Author

Just going through my own stale PRs, bump @matthewcn56; what do you want to do about this?

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