Skip to content
This repository has been archived by the owner on Mar 31, 2021. It is now read-only.

feat: turn into up-to-date CRA template #26

Merged
merged 14 commits into from
Dec 2, 2020
Merged

feat: turn into up-to-date CRA template #26

merged 14 commits into from
Dec 2, 2020

Conversation

felixjung
Copy link
Contributor

@felixjung felixjung commented Nov 27, 2020

Addresses #issue-number.

Purpose

This PR updates all SumUp dependencies and turns the project into a custom CRA template. With a CRA template users will be able to bootstrap an app by running yarn create react-app --template @sumup. Please refer to the updated README for details.

Definition of done

  • Development completed
  • Reviewers assigned

BREAKING CHANGE: This change is similar to a complete rewrite.

Copy link
Contributor

@herberthenrique herberthenrique left a comment

Choose a reason for hiding this comment

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

Amazing job @felixjung !

Can we move this project to the sumup-oss ?

@felixjung
Copy link
Contributor Author

felixjung commented Nov 27, 2020

Friends, could you please take this for a spin? To do so, check out the branch and run

yarn create react-app test-react-app-final --template file:create-sumup-react-app

where create-sumup-react-app should be the relative path to where you cloned this project.

There is a slight issue with linting. 😓 Because we want to provide a foundry linter config to the bootstrapped project, ESLint picks it up from template/.eslintrc.js. There seems to be no flag to disable this behavior, though. Any ideas?

@felixjung
Copy link
Contributor Author

I fixed the ESLint issues and also fixed the test template. Would be good if you gave it another try. Should all work as expected now.

.eslintrc.js Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
template/prettier.config.js Outdated Show resolved Hide resolved
template/prettier.config.js Outdated Show resolved Hide resolved
template/public/manifest.json Outdated Show resolved Hide resolved
Comment on lines +8 to +12
// If you want to start measuring performance in your app, pass a function
// to log results (for example: reportWebVitals(console.log))
// or send to an analytics endpoint. Learn more: https://bit.ly/CRA-vitals
reportWebVitals();
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is a CRA feature, right? Would there be a benefit in using our own signal library instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is part of CRA. I think it would be cool to use signal. Would there be any restrictions on how that can be used and how it needs to be configured?

Copy link
Member

Choose a reason for hiding this comment

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

Hm, yes, the configuration is more involved than I thought.

TL;DR: You need to generate your own script and inline it.

template/src/setupTests.js Show resolved Hide resolved
@felixjung
Copy link
Contributor Author

OK, by opting for running foundry init in a postinstall hook, I think we lost the ability to easily test this e2e. I've tested locally runing start, build, test, and create:component together with test. Everything worked. @connor-baer could you take another look?

Copy link
Member

@connor-baer connor-baer left a comment

Choose a reason for hiding this comment

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

@felixjung felixjung merged commit 0c842d7 into master Dec 2, 2020
@felixjung felixjung deleted the cra-template branch December 2, 2020 16:09
@ghost
Copy link

ghost commented Dec 2, 2020

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Dec 2, 2020
@connor-baer connor-baer linked an issue Mar 31, 2021 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Turn into custom CRA template
3 participants