Vendor create app template #1637

pepicrft · 2021-01-13T16:52:48Z

Having the create-redwood-app template in a different repository adds some indirection that can be avoided. This PR adds the template to the create-redwood-app package under /template and updates the generation code to copy the content from there instead of downloading it from GitHub releases. Thanks to this change it's easier to test template changes since it doesn't require adjusting the logic to read the template from a different source.

In a follow-up PR I'd like to add a few tests and start adding support for generating the mobile side.

packages/create-redwood-app/src/create-redwood-app.js

github-actions · 2021-01-13T16:57:46Z

📦 PR Packages

Click to Show Package Download Links

Install this PR by running yarn rw upgrade --pr 1637:0.23.0-ef01939

packages/create-redwood-app/package.json

packages/create-redwood-app/src/create-redwood-app.js

peterp · 2021-01-14T05:43:22Z

This is great @pepibumur!

So we're failing the lint check on the Routes.js file because the NotFoundPage is undefined. We automatically import the pages via a babel plugin. I think it's OK to disable this rule for this file; you can do that by appending something like this (I am unsure about the path matcher) to the .eslintrc file:

  "overrides": [
    {
      "files": [
        "*/templates/web/src/Routes.*"
      ],
      "rules": {
        "no-undef": "off"
      }
    }
  ]

Tobbe · 2021-01-14T07:00:54Z

Does this have any implications for the all-contributors stuff? @thedavidprice?
(Anyone who contributed to the crwa repo got a "🔧 tooling" contributor attribution. If this removes the need for the crwa repo, how do we handle that stuff?)

pepicrft · 2021-01-14T16:49:00Z

@thedavidprice / @peterp I followed your suggestion to disable the rule in the template directory and after trying different wildcards, I realized that eslint was not reading the configuration. Then I extended the lint script in package.json to pass -c .eslintrc and the command then picked the configuration. Am I doing something wrong?

peterp · 2021-01-17T09:05:37Z

@pepibumur I'm checking this out now - if you have some time would you push up your changes?

peterp · 2021-01-17T10:07:14Z

@pepibumur This is now resolved... turns out we never used the configuration file 🤦‍♂️

peterp · 2021-01-17T19:10:04Z

@Tobbe I removed all the ESLint warnings in this PR 😆

Tobbe · 2021-01-17T19:52:22Z

"@typescript-eslint/no-explicit-any": "off", you mean? That's one way to do it... 😄

pepicrft · 2021-01-18T17:06:20Z

@pepibumur This is now resolved... turns out we never used the configuration file 🤦‍♂️

Ouch. Thanks for looking into it @peterp. Do I have to do anything else on my side?

thedavidprice

Artifact files

If this is no longer a Repo, the following changes can be made:

delete template/.gitignore
rename template/.gitignore.app to /template/.gitignore
delete template/README.md
rename template/README_APP.md to template/README.md
remove title: 'Clean up task block starting L96 in src/create-redwood-app.js

Misc To-Dos

update the package create-redwood-app/README.md --> maybe easier to create a new issue and assign to me @thedavidprice
@peterp during NPM publishing, we'll need to somehow include a step where the @redwoodjs packages in the respective template/*/package.json are upgraded to the version being published, yes?

thedavidprice · 2021-01-19T03:48:24Z

Nice work on this one @pepibumur! I've added some review changes separately. Discussion & questions below:

Discussion

1. Test(s)

@pepibumur were you thinking about adding a test for create-redwood-app.js? There is a way to test the tasks/steps in Listr. But even if we just tested the first step copying the template to a directory it would be helpful for the CI running on Linux and Windows. There's not much going on otherwise.

Either way, we should run a Windows test before releasing (assuming Pedro isn't on Windows). 'Cause Windows + Paths always seem to bite us.

2. Possible to Manually Setup?

In the past, there have been occasions where people could not get create yarn app ... to work (perhaps firewall). So we directed them to a manual "download from repo" and setup process. How might something equivalent work in this case?

direct people to download repo and copy files from template/?
"download" from NPM and copy from package?
🤔

3. Incrementing Template Release with `-` or `+`

Although we are SuperAmazing™ at this, on occasion we have needed to cut a new Template release without incrementing release version. E.g. for sanity, we always keep the CRWA Template version the same as the included @redwoodjs package.json versions; if the latest Redwood release is 0.22.1, then the Template is at 0.22.1 as are all @redwoodjs versions in the respective package.jsons. However, there have been instances where we needed to make a change to the Template and created a release that is still at the same major.minor.patch version. We've used - notion here (probably incorrectly per semver) and + notation here (also not exactly sure about semver). Thoughts about these @peterp?

If needed, to publish the create-redwood-app package only we just need to modify the Lerna publish command. Easy peasy, correct?
But how can/should we handle semver in this case:
- without conflicting with auto-publishing of Canaries
- or messing up versioning for cutting the next release of all packages
- in a way where NPM will recognize it as the latest release to be used when people run yarn create ... — maybe this just means we always need to publish this type of release with latest tag despite semver?
- and, because yarn create ... installs the package globally (correct?), we don't run into local conflict where someone has previously run yarn create ... on version 0.23.1 and then tries again for something like 0.23.1+fixed-an-oops (or 0.23.1-fixed-an-oops). In either case, I have no idea what will happen locally in the case of global installs.

I like us to have a plan documented and have tested (somehow) beforehand to make sure we're not caught off guard. I'm game to go through a release cycle if needed 'cause pre v1, baby!

thedavidprice · 2021-01-19T03:49:05Z

looping in @aldonline --> given your refactoring of Structure package, I thought this PR might be of interest and, hopefully, helpful! ('Cause using it as fixture for Structure tests or something like that...)

peterp · 2021-01-19T07:04:00Z

@thedavidprice

Test(s)

Let's manually test this on Windows. Nothing sticks out to me as problematic. This is way simpler than before, and we're not changing much.

Possible to Manually Setup?

Those issues should not exist anymore. If you can run yarn create redwood-app then it would have downloaded the files.

Incrementing Template Release with

This is still possible. Instead of using lerna to release we'll change the version and use npm publish. We should really make this an automated process though.

The exact steps would be:

cd packages/create-redwood-app
bump version by editing package.json
yarn build
npm publish

peterp · 2021-01-19T07:05:48Z

@pepibumur I'll quick run through @thedavidprice's suggested changes. I'm pretty eager to get this done, since it's blocking some work that @Tobbe is doing on stripping out Apollo's packages.

Tobbe · 2021-01-19T08:47:39Z

@peterp I'm on standby to (manually) test this on Windows. Just ping me.

Tobbe · 2021-01-19T08:55:59Z

However, there have been instances where we needed to make a change to the Template and created a release that is still at the same major.minor.patch version.

For my own curiosity and opportunity to learn -- When/why was this needed? When can't we just do a patch-release of everything when the template needs to be updated?

peterp · 2021-01-19T09:46:44Z

@Tobbe

When/why was this needed?

We had a situation where the create-react-app template would be broken, but the packages were not. We did not want to republish all the packages, and just wanted to tag a new release in the create-redwood-app repo.

Tagging a release in the repo would mean that it was the template that was downloaded with yarn create redwood-app.

But, you're right, for the sake of simplicity, if the template is broken we should bump and release all the packages again.

Tobbe · 2021-01-19T10:01:08Z

But, you're right, for the sake of simplicity, if the template is broken we should bump and release all the packages again.

Yeah, automate the release process more, and it'll be easier to just cut a new patch release for everything. Which is the correct thing to do looking at SemVer. Make it easier to do the right thing! 🙂

thedavidprice · 2021-01-19T16:45:53Z

@peterp
Thanks for going through my list! Overall in agreement for sure. Some specifics:

1. Tests

Makes sense for getting this one out the door. And, once it's merged, this sets us up for improving CI by setting up a GitHub action that runs the Integration test using canaries after a PR is merged. I suggest this is a high priority and will create an Issue now and add to v1 Roadmap.

2. Manual Setup

I'm also assuming the problem (encountered by few that I know) will resolve if not go away entirely. I'm anticipating the question hitting our forums, though. Will figure it out if and when...

3. Incrementing Template Release (aka only publishing CRWA package)

Publishing steps look great and make sense. However, maybe this is the answer to my question, which is based on previous practices --> once the CRWA template is merged into main repo, we will NEVER publish the CRWA package separately as a non-patch, incremental release. Instead, we will always publish all packages together at same semver release.

Seems like the way to go, yes? (Thanks for letting me think through it here.)

thedavidprice · 2021-01-21T17:39:21Z

@peterp has this item been addressed? I am assuming it's not automatically handled by Lerna — maybe it is?

during NPM publishing, we'll need to somehow include a step where the @redwoodjs packages in the respective template/*/package.json are upgraded to the version being published, yes?

peterp · 2021-01-21T17:52:44Z

@thedavidprice We would have to run yarn rw upgrade in the template directory; but we should take care of that when we automate the releases.

thedavidprice · 2021-01-21T19:03:52Z

This might be where I'm confused, but I think we'd run into an infinite loop:

publish packages (which now includes CRWA template)
then go into Template to upgrade packages and yarn.lock...
...which, once upgraded, will require publishing (back to step 1)

Based on this understanding, it seems we'll need to handle changing the Template package versions at the same step as the Lerna publishing.

peterp · 2021-01-21T20:05:04Z

Ah lol, you're right. Maybe we need some TENET action. I'll write a script that updates this and the other packages: #1661 1661

Pedro Piñera added 2 commits January 13, 2021 17:37

Add template

3bb740a

Vendor the template inside the package and read it from there

349eb7d

pepicrft commented Jan 13, 2021

View reviewed changes

packages/create-redwood-app/src/create-redwood-app.js Show resolved Hide resolved

pepicrft commented Jan 13, 2021

View reviewed changes

packages/create-redwood-app/src/create-redwood-app.js Show resolved Hide resolved

Merge branch 'main' into vendor-create-app-template

64ec96f

pepicrft mentioned this pull request Jan 13, 2021

[Meta] Support for mobile #1623

Closed

6 tasks

thedavidprice self-requested a review January 13, 2021 18:59

peterp reviewed Jan 14, 2021

View reviewed changes

packages/create-redwood-app/package.json Show resolved Hide resolved

peterp reviewed Jan 14, 2021

View reviewed changes

packages/create-redwood-app/src/create-redwood-app.js Outdated Show resolved Hide resolved

peterp added 5 commits January 17, 2021 11:31

Remove legacy ignore rule.

ac70a3f

Explicitly specify ESLint configuration.

03afcb5

Define ESLint rule overrides in a single place.

bb2b9a8

Remove semi-colons.

7782228

Fix last few ESLint warnings.

a32b8b4

peterp added next release labels Jan 17, 2021

peterp added this to the next release milestone Jan 17, 2021

thedavidprice requested changes Jan 19, 2021

View reviewed changes

thedavidprice mentioned this pull request Jan 19, 2021

Add E2E Integration tests to CI #1658

Closed

peterp added 4 commits January 21, 2021 18:01

Merge branch 'main' into vendor-create-app-template

78d995c

Remove files from older repo.

92e7061

Move the check for emptyness inside the task block.

90592fa

Merge branch 'main' into vendor-create-app-template

ef01939

peterp merged commit ce645a6 into redwoodjs:main Jan 21, 2021

thedavidprice added the next release docs label Jan 21, 2021

peterp mentioned this pull request Jan 22, 2021

Why the default template is not in the monorepo? #419

Closed

thedavidprice mentioned this pull request Jan 25, 2021

Update React to the latest version in create-redwood-app redwoodjs/create-redwood-app#125

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vendor create app template #1637

Vendor create app template #1637

pepicrft commented Jan 13, 2021 •

edited

Loading

github-actions bot commented Jan 13, 2021 •

edited

Loading

peterp commented Jan 14, 2021

Tobbe commented Jan 14, 2021 •

edited

Loading

pepicrft commented Jan 14, 2021

peterp commented Jan 17, 2021

peterp commented Jan 17, 2021

peterp commented Jan 17, 2021

Tobbe commented Jan 17, 2021

pepicrft commented Jan 18, 2021

thedavidprice left a comment •

edited by peterp

Loading

thedavidprice commented Jan 19, 2021

thedavidprice commented Jan 19, 2021 •

edited

Loading

peterp commented Jan 19, 2021

peterp commented Jan 19, 2021

Tobbe commented Jan 19, 2021

Tobbe commented Jan 19, 2021

peterp commented Jan 19, 2021

Tobbe commented Jan 19, 2021

thedavidprice commented Jan 19, 2021

thedavidprice commented Jan 21, 2021

peterp commented Jan 21, 2021

thedavidprice commented Jan 21, 2021 •

edited

Loading

peterp commented Jan 21, 2021 •

edited

Loading

Vendor create app template #1637

Vendor create app template #1637

Conversation

pepicrft commented Jan 13, 2021 • edited Loading

github-actions bot commented Jan 13, 2021 • edited Loading

📦 PR Packages

peterp commented Jan 14, 2021

Tobbe commented Jan 14, 2021 • edited Loading

pepicrft commented Jan 14, 2021

peterp commented Jan 17, 2021

peterp commented Jan 17, 2021

peterp commented Jan 17, 2021

Tobbe commented Jan 17, 2021

pepicrft commented Jan 18, 2021

thedavidprice left a comment • edited by peterp Loading

Choose a reason for hiding this comment

Artifact files

Misc To-Dos

thedavidprice commented Jan 19, 2021

Discussion

1. Test(s)

2. Possible to Manually Setup?

3. Incrementing Template Release with - or +

thedavidprice commented Jan 19, 2021 • edited Loading

peterp commented Jan 19, 2021

peterp commented Jan 19, 2021

Tobbe commented Jan 19, 2021

Tobbe commented Jan 19, 2021

peterp commented Jan 19, 2021

Tobbe commented Jan 19, 2021

thedavidprice commented Jan 19, 2021

1. Tests

2. Manual Setup

3. Incrementing Template Release (aka only publishing CRWA package)

thedavidprice commented Jan 21, 2021

peterp commented Jan 21, 2021

thedavidprice commented Jan 21, 2021 • edited Loading

peterp commented Jan 21, 2021 • edited Loading

pepicrft commented Jan 13, 2021 •

edited

Loading

github-actions bot commented Jan 13, 2021 •

edited

Loading

Tobbe commented Jan 14, 2021 •

edited

Loading

thedavidprice left a comment •

edited by peterp

Loading

3. Incrementing Template Release with `-` or `+`

thedavidprice commented Jan 19, 2021 •

edited

Loading

thedavidprice commented Jan 21, 2021 •

edited

Loading

peterp commented Jan 21, 2021 •

edited

Loading