Full archetype including builder-init sample app #1
Conversation
zachhale
changed the title
| @@ -0,0 +1 @@ | |||
| node_modules | |||
There was a problem hiding this comment.
add dist
edit: never mind, that is only for init/{{gitignore}}
There was a problem hiding this comment.
You need a full .gitignore here that applies to all the files in the archetype, so like init, config, etc.
Should probably at least have:
\.git
\.hg
\.DS_Store
\.project
node_modules
npm-debug\.log*
phantomjsdriver\.log
coverage
There was a problem hiding this comment.
@ryan-roemer what's the reasoning for the escaped .s?
There was a problem hiding this comment.
@zachhale -- Long habit. But I don't remember my escaping / globbing rules where. I'd consult a modern gitignore reference and check -- the slashes may well be not needed.
|
Looks great! If you're a reviewer testing this out, you would run Then |
|
Looks like you need to add |
|
@zachhale and remove the |
| loaders: [ | ||
| { | ||
| test: /\.jsx?$/, | ||
| exclude: [/node_modules/], |
There was a problem hiding this comment.
Could we do path.join(ROOT, "node_modules/") here, too?
There was a problem hiding this comment.
Actually, I'd recommend removing exclude
and instead doing:
include: [base.context]
| @@ -0,0 +1,40 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
Remove entire file as it's not needed anymore. Yay!
| } | ||
| return res.json(); | ||
| }) | ||
| .then((data) => data) |
There was a problem hiding this comment.
Always catch even if you just explicitly re-throw
There was a problem hiding this comment.
The promise is returned. It's upstream responsibility to catch
There was a problem hiding this comment.
One question though - what's the point of the then(x => x)? This line should go away, yes?
There was a problem hiding this comment.
There was a problem hiding this comment.
Good catch, removing this line.
|
@baer responded to last round of feedback. Please re-review. |
|
@ryan-roemer is this clear for merge then? I'd love to get this in and start opening up separate issues if it's good as a v1! |
|
Merge it! If there is more work to be done lets do it in PRs so we can track it. Nice work Zach! |
Full archetype including builder-init sample app
This is a big one. /cc @coopy @ryan-roemer
builder-init-able sample app for demonstrating use of all of the abilities of this new archetypebuilder-init sample app
Features
react-routerimplementationreduxI tried to simplify the app down to a barebones including the above features inspired by @coopy's example app and in a way that I would want when starting a new react app. There are some features of
converter-reactfor bootstrapping through the querystring which I omitted. I also found the middleware model for bootstrapping data confusing for an app that has many routes that require differing bootstrapped data per route.Testing this PR
Once the project is generated, you'll need to npm link to the builder-react-app as well.
Then you can
npm installin peace!TODO
peerDependencies