feat(create-cycle-app): support motorcycle and ava #45

kristianmandrup · 2016-11-28T15:45:25Z

[x ] I ran npm test for the package I'm modifying
[x ] I used npm run commit instead of git commit
[x ] I have rebased my branch onto master before merging

Select core cycle lib and test runner. Adds infrastructure for passing multiple libs and use of
templating.

Select core cycle lib and test runner. Adds infrastructure for passing multiple libs and use of templating.

Fix lint and other small errors

kristianmandrup · 2016-11-28T15:47:26Z

Note, I was not able to test it locally.
I've tried linking it via npm link and installing via npm install . -g but have been unable to run it.

16:46 $ create-cycle-app demo
-bash: create-cycle-app: command not found

Please advice

geovanisouza92 · 2016-11-28T21:51:54Z

Hi @kristianmandrup, thanks for you contribution! I'll leave some comments as we discuss about this.

geovanisouza92

It's an awesome addition to CCA, @kristianmandrup, but it will be easier to get these changes merged with smaller (and well discussed) PR's.

geovanisouza92 · 2016-11-28T21:52:56Z

create-cycle-app/CHANGELOG.md

@@ -1,3 +1,14 @@
+# v2.1.0 (2016-11-28)


There isn't the need to change the CHANGELOG.md (as counter-intuitive as this seems), because it is updated by the release pipeline. So please, remove this change.

geovanisouza92 · 2016-11-28T21:54:43Z

create-cycle-app/index.js

  if (flavor) {
    // Ask just for the stream library
-    inquirer.prompt([streamLibQuestion]).then(function (answers) {
-      preparePackageJson(appFolder, appName, flavor, answers.streamLib, verbose)
+    inquirer.prompt([cycleCoreLib, streamLibQuestion, testLibQuestion]).then(function (answers) {


I liked this idea, but considering the philosophy of CCA (one of the key points is the focus for beginners), we prefer to keep things simple and small, so, I'll ask you to put this changes around test library choices in another PR.

mmm, although a nice idea testLibQuestion I'm not sure I see this as something that should be part of CCA as is not an option related to cycle itself (while the others definitely support diversity)

Then why have a test lib at all? where to draw the line?

It's a very good question. Considering recent efforts made by @Widdershin on https://github.com/cyclejs/time, we could consider expanding the default tests examples, showing the basics of how to test cycle components (at least for the current "Hello world" component)

geovanisouza92 · 2016-11-28T21:55:14Z

create-cycle-app/package.json

@@ -1,6 +1,6 @@
 {
  "name": "create-cycle-app",
-  "version": "2.0.0",
+  "version": "2.1.0",


There isn't need to change the number, as release pipeline update it too.

geovanisouza92 · 2016-11-28T21:56:59Z

cycle-scripts-es-browserify/package.json

@@ -19,6 +19,7 @@
    "cycle-scripts": "./index.js"
  },
  "dependencies": {
+    "ava": "^0.17",


So, I think that you are hardcoding ava as a dependency, like mocha. As your proposal is to make this configurable, we should consider remove the hard-dependency and make them peerDependencies, and install the lib choosed by the user together with other dependencies.

Because I saw no other easy way to add ava with current very inflexible architecture, besides creating a set of new duplicate projects for all combinations. Hardly the way to go about it.

You see inflexibility where I see openness. There isn't the need to duplicate each core flavor, because you could create new ones and publish them on npm independently.

Each cycle-scripts-* you see here, could be copied into motorcycle-scripts-* (or forked) and customized for motorcycle (you could just ignore cycle-specific options on init script). There you could change test libs, template files and everything else you see suitable as motorcycle defaults.

I'm not rejecting changes on the tool, I'm just arguing that there's easier ways to achieve the same results without bloating the core tool and flavors.

Ok, so, I see 4 valuable proposals until now:

Including @cycle/isolate as a default dependency

Adding motorcycle flavors

Making the test library/tool an option

Templating files

My proposal is that we turn each of this points into issues and build them progressively. What do you think?

geovanisouza92 · 2016-11-28T21:58:25Z

cycle-scripts-es-browserify/package.json

@@ -27,6 +28,7 @@
    "chalk": "^1.1.3",
    "cross-spawn": "^4.0.2",
    "dotenv": "^2.0.0",
+    "ejs": "^2.5.2",


I cannot agree with this in any manner. Installing a devDependency on the user's project just to render one example test file?

No, I think that this could be solved with two files and a single if inside init script.

I didn't know how else to add it. I didn't quite understand your nested projects infrastructure. Maybe you could document how to add the tools to do the work?
I've never seen anything quite like it. Loads of duplication to maintain in this way...
I always just have a single set of project templates... I developed dozens of similar generators in the past and never had to resort to this.

I agree that adding some more contribution docs would be really helpful.

BTW there is one question to be made: "Is this dependency used only to create the project or it's a project dependency itself?"

When is the first case, we should find simple (hacky) ways to do it on init scripts. Otherwise, it's a dependency.

Each flavor' dependency becomes a devDependency when the project is "ejected", so, if the dependency would be put in devDependencies on a normal project, it must be on flavor dependencies.

geovanisouza92 · 2016-11-28T22:00:45Z

cycle-scripts-es-browserify/scripts/init.js

+    cycle: ['@cycle/dom', 'xstream'],
+    motorcycle: ['@motorcycle/core', '@motorcycle/dom']
+  }
+  var extras = ['immutable', '@cycle/isolate']


So, what's the point in installing immutable by default? @cycle/isolate seems to be a recurrent case, so, this could be included on all flavors in a proper PR, but immutable seems too opinionated to be in the core flavors.

Usually I prefer to use ramda, that is a totally different lib, but serves me for similar cases. But again, this is my preference, not the end-user.

💯 on including @cycle/isolate .
Agree that immutable should still a user choice and a default of a core-flavor

geovanisouza92 · 2016-11-28T22:02:35Z

cycle-scripts-es-browserify/scripts/init.js

  var ownPackageName = require(path.join(__dirname, '..', 'package.json')).name
  var ownPath = path.join(appPath, 'node_modules', ownPackageName)
  var appPackageJson = path.join(appPath, 'package.json')
  var appPackage = require(appPackageJson)

+  if (libs.test === 'ava') {
+    appPackage.ava = {


There is another way to pass this parameters to the test runner? If possible, we should avoid polluting package.json with configuration that could be kept inside the cycle-scripts-* and put them only after eject.

geovanisouza92 · 2016-11-28T22:08:27Z

@kristianmandrup About the tests, you could run it with:

node create-cycle-app/index.js <name> --flavor ./cycle-scripts-es-browserify

kristianmandrup · 2016-11-28T22:17:36Z

Hi geovani,

Haha ;) About the tests, I just thought about that as well...

About ava, not from what I can see in the docs and my own experience. Any ava use would be used to config in package.json - it is the new trend for many new such dev tools to avoid polluting the project with too many config files and instead have these settings in one place :P

Immutable seems to be a common choice for motorcycle in the example apps I have seen. Could be a confirm question or in a list of common extra libs choices? I also like Ramda, in fact I've recently been looking into fantasyland which is a lib for full FP style programming with all the magic gems!

Ideally the package.json etc. could/should be created via EJS templates, and not the current hacky patching strategy IMO. You are free to make whatever changes to my contribution that you like.
At least if we can get motorcycle and ava test lib options it would be awesome!
You could also consider adding lint support, fx. standard, xo or eslint

geovanisouza92 · 2016-11-28T22:41:40Z

Your welcome. All these questions are valuable... for discussion with the community and small PR's.

I don't know ava, nor it's advantages against mocha, so, I would like to discuss this in another PR if possible.

If immutable is a common choice, it's likely that users will immediately install it after creating the project (like isolate, that is a even more common lib), like the current approach, that wasn't reported as a problem. But I'm certain that I could be wrong, so, discuss this with community is even more valuable, both for motorcycle and cycle flavors.

I still don't see the advantage in using a template engine to render 3 or 4 files. It's hacky, absolutely, and I'm still troubled with two npm install that are made currently, and would love to see some ideas about how we can solve this, with a nice and simple idea.

This was a thought decision since the day-one. I came to the conclusion that including a template system, plus a extensive question-answer interface, would not differ from existing tools, like yeoman, e.g.

Why don't use a yeoman generator then? There already have generators for cycle.

Why not create a even more customizable and opinionated tool? There already are one.

Each one (including CCA) has it's own approach to reach the same goal: help developers in getting the project setup quickly.

The approach here is more simplistic and concise, but we let all doors open for contribution (there is an automatic discovery system for community flavors).

If the goal was add every tool that we find useful on a large scale, thousand-lines project, we would bloat each "hello world" project with hundreds of MB of dependencies that could end not be used at all. Cycle's philosophy (at least, from my point of view), favors composition and incremental enhancement. Why it's tools should differ from this?

nickbalestra · 2016-12-02T11:52:49Z

As @geovanisouza92 mentioned and as we added in the CONTRIBUTING doc as a first point, opening an issue to discuss before opening a PR is always the best way to go especially in cases like this where is not just a bugfFix. I would suggest @kristianmandrup and @mandricore to open issues for all the interesting points and question emerged as we don't have clear answers yet. Moving the discussion there will also make it easier for others in the community to follow and participate.

nickbalestra · 2017-01-30T19:23:22Z

Closing this.
Added some of the suggestion as issues to be discussed further there.

kristianmandrup added 2 commits November 28, 2016 16:32

feat(create-cycle-app): support motorcycle and ava

16db788

Select core cycle lib and test runner. Adds infrastructure for passing multiple libs and use of templating.

fix(create-cycle-app): Fix lint errors

fea5640

Fix lint and other small errors

geovanisouza92 requested changes Nov 28, 2016

View reviewed changes

geovanisouza92 self-assigned this Nov 28, 2016

geovanisouza92 added the enhancement label Nov 28, 2016

This was referenced Jan 30, 2017

including @cycle/isolate as a default dependency #69

Open

Templating files #70

Closed

nickbalestra closed this Jan 30, 2017

perjerz pushed a commit to perjerz/create-cycle-app that referenced this pull request Nov 12, 2018

added cycle-hmr-example (cyclejs-community#45)

4647a69

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(create-cycle-app): support motorcycle and ava #45

feat(create-cycle-app): support motorcycle and ava #45

kristianmandrup commented Nov 28, 2016

kristianmandrup commented Nov 28, 2016

geovanisouza92 commented Nov 28, 2016

geovanisouza92 left a comment

geovanisouza92 Nov 28, 2016

geovanisouza92 Nov 28, 2016

nickbalestra Dec 2, 2016

mandricore Dec 2, 2016

geovanisouza92 Dec 2, 2016

geovanisouza92 Nov 28, 2016

geovanisouza92 Nov 28, 2016

mandricore Dec 2, 2016

geovanisouza92 Dec 2, 2016 •

edited

Loading

geovanisouza92 Nov 28, 2016

mandricore Dec 2, 2016

geovanisouza92 Dec 2, 2016

geovanisouza92 Nov 28, 2016

nickbalestra Dec 2, 2016

mandricore Dec 2, 2016

geovanisouza92 Nov 28, 2016

geovanisouza92 commented Nov 28, 2016

kristianmandrup commented Nov 28, 2016

geovanisouza92 commented Nov 28, 2016

nickbalestra commented Dec 2, 2016

nickbalestra commented Jan 30, 2017

feat(create-cycle-app): support motorcycle and ava #45

feat(create-cycle-app): support motorcycle and ava #45

Conversation

kristianmandrup commented Nov 28, 2016

kristianmandrup commented Nov 28, 2016

geovanisouza92 commented Nov 28, 2016

geovanisouza92 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geovanisouza92 Dec 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geovanisouza92 commented Nov 28, 2016

kristianmandrup commented Nov 28, 2016

geovanisouza92 commented Nov 28, 2016

nickbalestra commented Dec 2, 2016

nickbalestra commented Jan 30, 2017

geovanisouza92 Dec 2, 2016 •

edited

Loading