Compile coffeescript #30

jasonkarns · 2015-10-01T15:18:25Z

Resolves #29

searls · 2015-10-01T16:02:50Z

Would you mind removing changes to dist/ in this PR. I only want those to change on version bump commits

searls · 2015-10-01T16:03:48Z

package.json

@@ -8,40 +8,40 @@
    "email": "justin@testdouble.com",
    "url": "http://testdouble.com"
  },
-  "main": "index.js",
-  "browser": "lib/testdouble.coffee",


no more "browser" entry?

nope. the browser entry was only used to bypass the coffeescript registration being done in index.js. now that there is no coffeescript registration, the node entrypoint and browser entrypoint are the same.

fantastic

On Thu, Oct 1, 2015 at 2:38 PM Jason Karns notifications@github.com wrote:

In package.json
#30 (comment)
:

@@ -8,40 +8,40 @@
"email": "justin@testdouble.com",
"url": "http://testdouble.com"
},

"main": "index.js",

"browser": "lib/testdouble.coffee",

nope. the browser entry was only used to bypass the coffeescript
registration being done in index.js. now that there is no coffeescript
registration, the node entrypoint and browser entrypoint are the same.

—
Reply to this email directly or view it on GitHub
https://github.com/testdouble/testdouble.js/pull/30/files#r40949956.

searls · 2015-10-01T16:04:46Z

Looks pretty exhaustive

don't use .gitignore git ignores generated js, but we need generated js in the package git includes coffee but we don't want coffee in package git includes tests but we don't want tests in package

coffee already implicitly compiled before published because we use npm-version (which does a build which does a compile) is what kicks off a publish step. However, publish is also invoked whenever a package is installed at the top level. (Such as when `npm install` is run inside of a cloned repo) Compilation is necessary in this case so that a single npm-install will result in a requireable package. This is precisely why npm runs publish on `npm install` (with no args)

jasonkarns · 2015-10-01T18:57:59Z

The package.json diff looks bigger than it is.

clean is the same as before except i moved it to the top (and swapped tmp for generated)
test is the same except with long switch names instead of short flags
test:ci is the same except reports all results in TAP
build is the same except i use full switch names, and no longer register the .coffee extension

Bigger picture:

coffee source is compiled before a browserify build so browserify doesn't do the coffee compilation itself (no more coffeeify)
thus both node and browser entrypoints are the same, which is the compiled JS under generated/
coffee source is compiled before publish
thus the main entrypoint exists when published to registry as well as when npm install is run with no args
coffee tests and source are compiled to generated/ before testem browser test runs thus browser tests point to generated/

mocha tests still point to coffee source, though this can be changed if desired.

The important thing to me is that the same coffee compiler is used to compile for distribution (npm run compile) as does the compilation under test. Mocha just delegates to the locally-installed coffee compiler so this holds. (It did not hold when browserify was using coffeeify, so coffeeify was ripped out.) We could go a step further and do a coffee compilation of source and tests prior to the mocha run, that way we know the tests are hitting the same compiled JS as will be distributed. As it stands now, it's using the same coffee compiler, but we're not guaranteed the same compiler settings.

jasonkarns · 2015-10-01T19:02:55Z

In order to see what the published tarball will look like:

ensure the working copy is clean
npm run clean
npm pack

the tarball should include:

bower.json
package.json
LICENSE
README
dist/testdouble.js
generated/*/.js

searls · 2015-10-01T19:11:53Z

+1

On Oct 1, 2015, at 14:57, Jason Karns notifications@github.com wrote:

The package.json diff looks bigger than it is.

clean is the same as before except i moved it to the top (and swapped tmp for generated)
test is the same except with long switch names instead of short flags
test:ci is the same except reports all results in TAP
build is the same except i use full switch names, and no longer register the .coffee extension
Bigger picture:

coffee source is compiled before a browserify build so browserify doesn't do the coffee compilation itself (no more coffeeify)
thus both node and browser entrypoints are the same, which is the compiled JS under generated/
coffee source is compiled before publish
thus the main entrypoint exists when published to registry as well as npm install with no args
coffee tests and source are compiled to generated/ before testem browser test runs thus browser tests point to generated/
mocha tests still point to coffee source, though this can be changed if desired.

The important thing to me is that the same coffee compiler is used to compile for distribution (npm run compile) as does the compilation under test. Mocha just delegates to the locally-installed coffee compiler so this holds. (It did not hold when browserify was using coffeeify, so coffeeify was ripped out.) We could go a step further and do a coffee compilation of source and tests prior to the mocha run, that way we know the tests are hitting the same compiled JS as will be distributed. As it stands now, it's using the same coffee compiler, but we're not guaranteed the same compiler settings.

―
Reply to this email directly or view it on GitHub.

searls · 2015-10-12T15:12:49Z

Is this ready to merge?

jasonkarns · 2015-10-12T15:20:40Z

I think so, though it's been sufficiently purged from my memory

searls · 2015-10-12T17:28:15Z

Mind pushing to a @next tag or something? Just added you as owner

jasonkarns · 2015-10-12T17:47:04Z

sure thing

On Mon, Oct 12, 2015 at 1:28 PM, Justin Searls notifications@github.com
wrote:

Mind pushing to a @next https://github.com/next tag or something? Just
added you as owner

—
Reply to this email directly or view it on GitHub
#30 (comment)
.

searls · 2015-10-24T12:04:50Z

merging because nobody knows if this works.

Compile coffeescript

compile script

c2f32c2

searls reviewed Oct 1, 2015
View reviewed changes

jasonkarns added 11 commits October 1, 2015 14:41

point main to js

56bd124

git should ignore compiled js

fdd3653

compile coffee source into generated/

34d517e

git-ignore generated js

8c472ab

coffee compilation must be done prior to browserify build

21b3813

compile:tests compiles test coffee, update testem

258f3fb

test:ci should emit TAP

2824293

no more index. main is generated/testdouble.js

1962ca9

coffeescript is now just a dev dep

9565747

manual npm ignore

1c69fdb

don't use .gitignore git ignores generated js, but we need generated js in the package git includes coffee but we don't want coffee in package git includes tests but we don't want tests in package

jasonkarns force-pushed the compile-coffee branch from cc52887 to ffb9753 Compare October 1, 2015 18:42

go ahead and include lib/ and test/ in the npm package

d2eefd5

searls added a commit that referenced this pull request Oct 24, 2015

Merge pull request #30 from testdouble/compile-coffee

5475693

Compile coffeescript

searls merged commit 5475693 into master Oct 24, 2015

jasonkarns deleted the compile-coffee branch November 18, 2015 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compile coffeescript #30

Compile coffeescript #30

jasonkarns commented Oct 1, 2015

searls commented Oct 1, 2015

searls Oct 1, 2015

jasonkarns Oct 1, 2015

searls Oct 1, 2015

searls commented Oct 1, 2015

jasonkarns commented Oct 1, 2015

jasonkarns commented Oct 1, 2015

searls commented Oct 1, 2015

searls commented Oct 12, 2015

jasonkarns commented Oct 12, 2015

searls commented Oct 12, 2015

jasonkarns commented Oct 12, 2015

searls commented Oct 24, 2015

Compile coffeescript #30

Compile coffeescript #30

Conversation

jasonkarns commented Oct 1, 2015

searls commented Oct 1, 2015

searls Oct 1, 2015

Choose a reason for hiding this comment

jasonkarns Oct 1, 2015

Choose a reason for hiding this comment

searls Oct 1, 2015

Choose a reason for hiding this comment

searls commented Oct 1, 2015

jasonkarns commented Oct 1, 2015

jasonkarns commented Oct 1, 2015

searls commented Oct 1, 2015

searls commented Oct 12, 2015

jasonkarns commented Oct 12, 2015

searls commented Oct 12, 2015

jasonkarns commented Oct 12, 2015

searls commented Oct 24, 2015