[CS2] Add webpack support #4501

akfish · 2017-04-14T14:15:32Z

Per discussions with @GeoffreyBooth, @timhuff and @connec in #4276 #4277 reported by @Radivarig, current coffee-script source cannot be packed with webpack due to references to Node.js-only modules in main script.

Example error output of webpack:

Error in ./~/coffee-script/lib/coffee-script/coffee-script.js
Module not found: 'module' in project_root\node_modules\coffee-script\lib\coffee-script

 @ ./~/coffee-script/lib/coffee-script/coffee-script.js 168:23-40 208:17-34

Error in ./~/coffee-script/lib/coffee-script/register.js
Module not found: 'module' in project_root\node_modules\coffee-script\lib\coffee-script

 @ ./~/coffee-script/lib/coffee-script/register.js 25:13-30

Error in ./~/coffee-script/lib/coffee-script/register.js
Module not found: 'child_process' in project_root\node_modules\coffee-script\lib\coffee-scrip

 @ ./~/coffee-script/lib/coffee-script/register.js 7:18-42

Error in ./~/coffee-script/lib/coffee-script/command.js
Module not found: 'child_process' in project_root\node_modules\coffee-script\lib\coffee-scrip

 @ ./~/coffee-script/lib/coffee-script/command.js 16:8-32

Error in ./~/coffee-script/lib/coffee-script/grammar.js
Module not found: 'jison' in project_root\node_modules\coffee-script\lib\coffee-script

 @ ./~/coffee-script/lib/coffee-script/grammar.js 5:11-27

Error in ./~/coffee-script/lib/coffee-script/repl.js
Module not found: 'repl' in project_root\node_modules\coffee-script\lib\coffee-script

 @ ./~/coffee-script/lib/coffee-script/repl.js 11:13-28

Changes

Move Node.js-only code from src/coffee-script.coffee to src/index.coffee
Use lib/coffee-script/index.js as npm package's "main" script
Export CoffeeScript from src/browser.coffee
Set package.json's "browser" field to lib/coffee-script/browser.js (used by webpack as entry point)
Use lib/coffee-script/browser.js as bower package's "main" script

Tests

cake build
cake build:browser
cake test
cake test:browser
Manually tested packing with webpack
Manually verified compiled browser bundle works in browser (with re-built doc site)

* Move Node.js-only code from src/coffee-script.coffee to src/index.coffee * Use lib/coffee-script/index.js as npm package's "main" script * Export CoffeeScript from src/browser.coffee * Set package.json's "browser" field to lib/coffee-script/browser.js (used by webpack as entry point) * Use lib/coffee-script/browser.js as bower package's "main" script

jashkenas · 2017-04-14T15:19:27Z

This is a lot of rejiggering for the webpack use case. CoffeeScript is primarily a Node.js compilation tool — not a browser tool. Our main should not be a browser.js file.

That said, a general reorganization of Node-specific parts to make webpack work sounds fine. Let's just keep the entry point for browser use as "browser".

GeoffreyBooth · 2017-04-14T15:29:01Z

src/coffee-script.coffee

-exports.run = (code, options = {}) ->
-  mainModule = require.main
+# Compile and execute a string of CoffeeScript 
+exports.run = (code, options = {}) -> throw new Error 'Not implemented'


Why export this if it just throws an error?

The idea is that coffee-script.coffee will contain only platform-independent code. Any platform-specific code should be moved elsewhere. Throwing an error in those "abstract" method prevents developers from accidentally require lib/coffee-script/coffee-script and not knowing what's wrong.

GeoffreyBooth · 2017-04-14T15:32:53Z

This will be tricky in 2. Take a look at cake build:browser in the 2 branch, where it includes the Markdown-It library.

akfish · 2017-04-14T15:42:51Z

@jashkenas The npm package's main is index.js, which points to node.js only implementation.

I modified bower package's main to browser.js, because bower describe itself as "A package manager for the web". I assume that implies any bower related usage should be browser-only. But I could be wrong because I don't use bower.

akfish · 2017-04-14T15:59:06Z

@GeoffreyBooth The Markdown-it library seems to be added via simple concatenation in branch 2's cakefile. The coffee-script part seems to be the same as master. We could basically apply the same diff to Line 146 of branch 2's Cakefile:

-        return require['./coffeescript'];
+        return require['./browser'];

akfish · 2017-04-14T16:22:50Z

@jashkenas A little follow up on bower.json's main field.

Bower Spec defines it as:

The entry-point files necessary to use your package. Only one file per filetype.
... Bower does not directly use main files, they are listed with the commands bower list --json and bower list --paths, so they can be used by build tools.

It suggests that build tools determine how to use "main" field.

webpack uses it as entry point. See: https://webpack.github.io/docs/usage-with-bower.html
browserify's debowerify transform plugin use it as entry point as well. See: https://github.com/eugeneware/debowerify#notes

So it should points to a browser-specific implementation for build tools to work properly.

jashkenas · 2017-04-14T16:44:22Z

This will be tricky in 2. Take a look at cake build:browser in the 2 branch, where it includes the Markdown-It library.

Kind of a good argument for us trying to leave out all dependencies, and just do the markdown code block detection ourselves.

After all — we don't need to parse 98% of Markdown. The only thing we need to parse is code blocks.

akfish · 2017-04-14T17:24:25Z

Just a thought: It would be trivial to setup webpack for building coffee-script browser bundle instead of cake build:browser after this PR is merged. Any dependencies can be easily resolved by webpack, as long as they play well with webpack.

And I happened to be using Markdown-it in my project too. It works well with webpack.

GeoffreyBooth · 2017-04-14T17:44:31Z

As @jashkenas implies, we’re not looking to add dependencies, so adding Webpack to our build is almost surely a nonstarter. The reason Markdown-It was added was to address false positives in detecting Markdown non-code blocks: #3924. Though the original issue didn’t have examples that failed, so I don’t know what would be involved in rolling our own implementation. That’s perhaps something we should revisit.

The docs do say:

The core compiler however, does not depend on Node, and can be run in any JavaScript environment, or in the browser (see Try CoffeeScript).

So we should really try to honor this promise, and disentangle the Node dependencies.

akfish · 2017-04-14T18:04:19Z

Webpack would be a devDependency, which is not the same as dependency.
But I understand that as a compiler, the more dependencies it has, the less "meta" it feels like.

Anyway it's just a possibility and off the topic of this PR. So just ignore me. :)

lydell · 2017-04-15T15:48:36Z

Markdown is very difficult to parse.

GeoffreyBooth · 2017-04-15T19:11:11Z

@akfish A few questions:

Currently the browser compiler is built and committed into the repo in docs/v1/browser-compiler/coffee-script.js or docs/v2/browser-compiler/coffeescript.js. This isn’t included in the Node module, but what if it was? And then we added a browser key to package.json that pointed to it? Could that solve your problem all by itself?
Can you please post a minimal repo showing Webpack using this PR? So that we can see that it works, and also that if we flip to the released 1.12.5 it doesn’t work; and also so that I can test alternatives like the idea above.
Is there a test we can write that verifies this? Not a test specific to Webpack, but rather that requiring the compiler doesn’t also mean that requiring fs succeeds etc. Perhaps the test or the test runner can override the require function, so that we have a way to fail the test if some portion of the compiler tries to require a Node module.

I think coming up with a good test for this is important. Years ago the compiler didn’t depend on Node, and clearly at some point that got lost; if we have a test that fails the moment someone puts fs = require 'fs' in the code, that will prevent such a thing from happening again.

akfish · 2017-04-15T21:34:54Z

For webpack, browser key serves as an entry script for package resolution. They looks for calls require or import calls recursively from there and do a custom bundle each time the developer builds his project. So it doesn't matter to webpack what other files are included. The pre-built bundles will not be used by webpack, as long as they are not required explicitly by any files along its resolution path. And it is generally not a good practice to point browser field to pre-built bundles. It's best to only provide an entry script and let webpack figure out what needs to be packed.
Will do.
Hook require could work. Although that means we would have to manually maintain a blacklist for non-compatible modules. And different tools resolves require calls differently (but mostly some form of static code analysis). The behavior might not be the same as runtime require hook. An easier and more reliable way is to run webpack build as a test. This approach requires adding webpack as devDependency. Add a webpack config file. Then simply run webpack lib/coffee-script/browser.js dist/bundle.js and see if there're any errors.

I'll cook up some example repos for 2 and 3.

akfish · 2017-04-15T23:18:32Z

Check out https://github.com/akfish/coffeescript/tree/demo/webpack-bundle. This repo demonstrates the minimal modifications (webpack as devDependency and a new webpack.config.js file) needed to test this PR by running a webpack build. Run node_modules/.bin/webpack after ran cake build will do the trick (or we could add a cake test:webpack task to run this command). I understand the the reluctance to add an additional dependency. But it's the cleanest method to test this PR that I could think of.

When running this test, I found out that generated parser.js contains reference to fs module, which messed things up. Commit 6cde4b5 fixed it by passing a NOP function to override the default exports.main emitted by Jison. As far as I could tell, that method was not called anywhere.

akfish · 2017-04-15T23:59:51Z

Here is a minimal demo showing webpack using CoffeeScript: https://github.com/akfish/coffeescript-webpack-demo
Also commit 665eba1 removed this line that broke the minimal demo (but oddly enough not my already-too-complex project). The commit added it 7 years ago said it's used for debugging and I didn't find any code references to it. And removing it didn't break any tests. I hope that's ok.

…ebpack

GeoffreyBooth · 2017-04-18T03:25:01Z

Cakefile

-          if (typeof fs !== 'undefined' && fs !== null)
-              source = fs"""
+  # We don't need moduleMain, since the parser is unlikely to be run standalone
+  parser = require('./lib/coffee-script/grammar').parser.generate(moduleMain: ->)


This is brilliant. Thanks for solving this better than my hack.

GeoffreyBooth · 2017-04-18T03:25:56Z

Cakefile

@@ -135,7 +129,7 @@ task 'build:browser', 'merge the built scripts into a single file for use in a b
      var CoffeeScript = function() {
        function require(path){ return require[path]; }
        #{code}
-        return require['./coffee-script'];
+        return require['./browser'];


Can you explain this? Why browser here?

This is for logic consistency. Since browser.js is already used as the entry for bower and webpack. It could avoid unnecessary confusion to use it for all browser related situations.

GeoffreyBooth · 2017-04-18T03:26:28Z

bower.json

@@ -1,7 +1,7 @@
 {
  "name": "coffee-script",
  "main": [
-    "lib/coffee-script/coffee-script.js"
+    "lib/coffee-script/browser.js"


This is fine. I’ve used Bower, it’s only for frontend (browser-based) JavaScript.

GeoffreyBooth · 2017-04-18T03:26:53Z

src/browser.coffee

@@ -4,7 +4,6 @@
 # `text/coffeescript` script tags, source maps via data-URLs, and so on.

 CoffeeScript = require './coffee-script'
-CoffeeScript.require = require


Thanks for catching this.

GeoffreyBooth · 2017-04-18T03:39:21Z

src/browser.coffee

@@ -18,6 +17,10 @@ CoffeeScript.run = (code, options = {}) ->
  options.shiftLine = on
  Function(compile code, options)()

+CoffeeScript.register = ->


So it seems like the strategy here is to declare this dummy method, and then in the Node version this gets overridden with the real method? This is confusing, to say the least.

This is confusing because register method should not be available in browser environment, thus it should not be one of the abstract method. But doing so will break browser test.

Because coffee's browser test is not actually ran in a browser. It runs in node environment and the first call in the runTest method is CoffeeScript.register(). I guess this might be because there's no karma, PhantomJs nor headless chromium back then.

That's why I set it to a NOP implementation to pass the test for now. I could configure a test script that actually runs in browser with one of the methods mentioned above. But I think that should be a separate PR/issue for discussion since it requires adding more devDependencies.

On a separate note, I also think runScripts method in the browser.coffee could be used as browser's register implementation. It's logically sound. But it will only work if the browser tests are actually running in browser. Right now it will only break the test when trying to call it in node environment.

GeoffreyBooth · 2017-04-18T03:52:34Z

What if we detected whether we’re in Node or not, and had the code branch accordingly? Or to follow the feature detection pattern, the functions that require Node modules could explicitly check for them and error out otherwise? Something like:

try
  fs     = require 'fs'
  path   = require 'path'
  module = require 'module'
  vm     = require 'vm'
catch (e)

exports.run = (code, options = {}) ->
  unless require?.main? and process?.argv? and fs? and path? and module?
    throw new Error 'This function requires CommonJS and the process, fs, path and module modules'
  # rest of old `run` method

And then we don’t need abstract or dummy methods, and the refactoring doesn’t need to be so dramatic. That would make me much more comfortable that we aren’t breaking downstream projects that might require the coffee-script module.

akfish · 2017-04-18T16:43:20Z

@GeoffreyBooth Tools like webpack use static analysis to find require calls. The code is not actually executed. Conditional require and feature detection are runtime solutions. For example, the parser's main method is clearly not executed, but it broke webpack.

With that said, abstract method is one of the common patterns to enforce or at least document shared interfaces/behaviors, should multiple implementations are required.

GeoffreyBooth · 2017-04-18T17:30:40Z

Tools like webpack use static analysis to find require calls.

Hmf. So it tries to resolve all require calls, before the Node runtime ever gets involved? So no matter where a require statement is in the file, Webpack tries to resolve it? So even something like this wouldn’t work?

# `isNode` defined above somehow

exports.run = if isNode then require('run') else ->

akfish · 2017-04-18T18:10:13Z

@GeoffreyBooth Yes. IIRC, webpack performs static analysis on AST. Older tools like browserify ~~uses simple RegExp match~~ analysis AST too. The code was never run.

GeoffreyBooth · 2017-04-19T06:22:44Z

What’s the argument for merging this into master instead of 2? I’m much less worried about breaking changes in 2.

I can see how having a browser-safe module you can include that generates ES3 is desirable; is that the appeal? I assume it wouldn’t be too difficult to configure Webpack to require CoffeeScript 2 (as modified by this PR) and pipe its output through Babel, even in a browser-compatible way?

akfish · 2017-04-19T13:29:37Z

I have no problem with that, as long as it'll will be published to npm.

I haven't had the need to generate ES3 yet so I can't say for sure. But it should be the case.

GeoffreyBooth · 2017-04-19T14:25:10Z

CoffeeScript 2 is already on NPM: npm install coffeescript@next. See http://coffeescript.org/v2/

So if that's okay with you, please target the 2 branch and revise accordingly.

GeoffreyBooth · 2017-05-10T06:59:39Z

Ideally we should bundle browser tests along with CoffeeScript and run the built script in an actual browser environment to test run-time behaviors.

That’s a good idea. We actually have tests to run in an real browser environment: test.html. These tests don’t run as part of any cake task, though—but they could. In another project, I’m using Selenium Chromedriver via Docker to run browser-based tests. We could have a cake task that checks if Docker is installed, and if it is, run a Docker command that fires up the Selenium Chromedriver image and loads our test.html, and waits to see if the tests all pass.

The current test suite runs in about 2 seconds on my machine, and the Webpack test runs in about 1 second. Obviously this Docker test would take considerably longer, so it might make sense to have separate testing tasks like:

cake test: same as current
cake test:browser: same as current
cake test:slow: run just the Webpack and Docker tests
cake test:full: run all of the above

I’m not asking you to write the Docker test. But let’s put the Webpack test inside a task named test:slow, so that we have a new place for such more-ambitious tests.

As to your point about the Webpack output or build report: in this project, passing tests are silent and failing tests dump copious debugging info. I think the Webpack test should follow that pattern. You can add a comment explaining how to get the Webpack output in all cases (presumably just by running the Webpack command, I assume). That should be enough.

So Webpack is generating this coffeescript.js in a temp folder, and this new file is essentially the same as the browser compiler? It can be required by another module, and it exports the CoffeeScript object with a compile method? Shouldn’t we continue the test to require the file that Webpack generates, and make sure it can actually compile some simple CoffeeScript into JavaScript? Or better yet, could we run the test suite against it, the way test:browser does?

akfish · 2017-05-10T11:37:21Z

Since webpack support is actually support for a new build target, rather than a new feature, how about keep it consistent with other two existing build targets (node and browser), by add two new tasks:

build:webpack (rename from current test:webpack, like build and build:browser)
test:webpack (run tests against webpack build, like test and test:browser)

In this way we can verify this new build target in two stages: 1) it should build and 2) built bundle should pass tests. Should the test configuration prove to be too much work to be complete in a single PR, we could at least have build:webpack for now and complete test:webpack in a separate PR.

akfish · 2017-05-10T12:23:54Z

As for the browser test, docker is too heavy for this in my opinion. A popular choice would be karma. I used it for my past projects. It only requires developers to have browser installed (and who doesn't). The test output is piped to console with proper exit code so it's great for automatic tests. You can run it in Travis CI too.

Though we will need to write some adapter code (or migrate tests to an existing test framework) for it to work, since this project doesn't use an existing one. The benefit is that we can run tests in actual browser environment, while sharing the same test suites among 3 build targets.

Apparently browser test setup is another can of worms to be opened and deserves a separate PR.

In fact we already have test coverage for webpack build from test:browser. Webpack build and build:browser both use browser.coffee as the entry point. They are essentially the same. I'll see if it's possible to require built bundle and run it like test:browser for now. But we're not completely in the blind if there're no dedicated tests for it.

akfish · 2017-05-10T13:18:32Z

I renamed test:webpack to build:webpack and added an actual test:webpack task. I found a way to load the built bundle and run tests suite against it in node environment, like test:browser did. In this way we have the same test coverage for webpack build target as the browser one, without getting into browser test setup.

GeoffreyBooth · 2017-05-10T16:21:33Z

What purpose does build:webpack serve? Do we have any reason to create that build other than to use for testing?

akfish · 2017-05-10T16:44:40Z

Logical consistency for one. Both node and browser build target follows the 2 stage build/test process, why not for webpack? It is a build target and should be verified in both stages.
Build and test are not the same. Build verifies static behaviors and test verifies run-time behaviors. They should not be mixed. Otherwise it's like running build:browser as a test.
Another reason is the build report. Build task can and should output human-readable report, while tests should be silent failing by this project's convention.

GeoffreyBooth · 2017-05-10T17:37:43Z

I’m trying to avoid creating more cake tasks. Each task should do something useful for people. Tasks with no clear purpose just muddy things.

It’s probably best to just have test:browser include what it does now, then build and test the Webpack build. The fact that the Webpack build is built in a temp folder and discarded afterward makes clear that its purpose is just for testing.

akfish · 2017-05-10T18:22:38Z

Testing a build target is not the same thing as testing a feature. Build report is not the same as test report and should not be consumed the same way. Discarding the build result doesn't make it less of a build target. And the webpack build result is not discarded: the build report is a build artifact and it is used.

It is common for projects to have build tasks but not to distribute the build artifacts. Many Linux packages only provide source code and let the user make on their own. Some npm packages don't have built js files in repo, only the source. And it's build on prepublish or postinstall hook.

A developer who's familiar with webpack would expect it to be treated as a build target. It could be more confusing to see it not to be treated as such, than having two tasks that keeps the logical consistency and can be easily documented. The logic is very clear: each build target would have build:*, test:* and build:*:full tasks. It's not muddy.

Speaking of which, maybe I should add a build:webpack:full task that invoke build:webpack and test:webpack, following the convention of the other two build targets. Wouldn't that achieve the "one-command test for webpack" goal while keeping the consistency?

GeoffreyBooth · 2017-05-10T18:41:37Z

A “webpack build” is not a feature of CoffeeScript, and I’m not interested in adding it as one. cake build builds a Node-based compiler, and cake build:browser builds a compiler that can run in browser environments; and both compilers are intended to be used by people. The compiler created by build:webpack is intended only for testing. I don’t want anyone to think otherwise.

Just because a test involves running a build command doesn’t mean it’s not still a test.

akfish · 2017-05-10T18:59:47Z

It's not a feature, it's a build target. Anyone familiar with webpack will not even come close to make the mistake you are thinking of.

Anyway, enough with the terminology stuffs, as long as the webpack build command is not run as a unit test and the build report is not silent, I'm happy with it. If that's acceptable for you, I will make the modifications so that there's only one cake task test:webpack that first builds a bundle then run the test suite against it.

jashkenas · 2017-05-10T20:48:09Z

Apologies. Looks like I should have helped put this ticket out of its misery long ago...

CoffeeScript already works just fine with Webpack, and should not be restructured like this. I just walked through webpack's tutorial, using the CoffeeScript "browser" compiler, and it works perfectly.

Using the latest version of CoffeeScript, you build the browser compiler:

cake build:browser

Copy it into your webpack project, and then use it:

import CoffeeScript from './coffee-script.js';

function component () {
  var element = document.createElement('div');

  element.innerHTML = CoffeeScript.compile('a = 1 + 1');

  return element;
}

document.body.appendChild(component());

Index.html:

<html>
  <head>
    <title>webpack 2 demo</title>
  </head>
  <body>
    <script src="dist/bundle.js"></script>
  </body>
</html>

Then you run webpack:

> ~/Desktop/webpack-demo webpack app/index.js dist/bundle.js

And it works:

@GeoffreyBooth -- if there's a change we need to make here, it's that we should be shipping the "browser" build file in a reasonable location as part of the npm install — you shouldn't have to build it yourself. Perhaps in a dist folder?

akfish · 2017-05-10T21:06:20Z

It works is not the same as it works "fine". I am aware of the setup you provided above. It shows how webpack supports a legacy project like this, not the other way around. The fact that the webpack support issue has been raised over and over by multiple people on multiple occasions apparently means nothing. Because people who use webpack on daily basis are neither too lazy to read the basic document, nor too stupid to know the difference. We should know better. Thanks for your time.

Radivarig · 2017-05-10T22:12:22Z

....unbelievable

GeoffreyBooth · 2017-05-10T22:12:52Z

Yes I think what @akfish is getting at is that you can’t just do CoffeeScript = require('coffeescript') in a Webpack or Browserify project and have it work. And not only that, there’s nothing like require('coffeescript/browser') that works. The browser build is not distributed over NPM, so there’s no way to require it.

Currently the browser build is treated as just part of the documentation, but clearly that needs to change. I think it deserves enough prominence that it is included in the NPM module, and require-able on its own.

What @akfish was accomplishing with this PR, which we might want to still consider, is that this PR enables people to do require('coffeescript') in Webpack/Browserify and it just works, the same as if they had typed that in Node. They wouldn’t need to specify require('coffeescript/browser') or similar. (@akfish, please correct me if I’m wrong about this.)

Maybe that’s not enough of an upside to be worth the changes contemplated by this PR, but as far as I can tell I don’t see any downside to these changes. It’s just a lot of lines of code moving around, but no functionality or clarity lost as a result. I guess if it provides a bit of convenience for Webpack/Browserify users, especially when it comes to time saved as they can avoid needing to troubleshoot errors caused by require('coffeescript') not working as expected until they discover they need to do require('coffeescript/browser') instead, why wouldn’t we make these changes?

jashkenas · 2017-05-10T22:25:08Z

Looking through the diff a little more, that sounds fair enough — as long as we aren't contorting the file structure in any webpack-specific way. Go for it. I'm sorry if I offended.

But we should still be offering the basic "coffeescript/browser" file as a general part of our packaging — so that folks who want to load it under a script loader of any stripe, or just vanilla JS, can get at it.

carlsmith · 2017-05-10T22:31:04Z

Clientside CoffeeScript is useful in some circumstances, even if it's normally a mistake to use it that way. If you're using CoffeeScript in the browser for the right reasons, then you may not even have Node set up in your development environment. This PR is more to do with how the compiler is modularised for browsers, but having first-class support for clientside CoffeeScript is worthwhile.

GeoffreyBooth · 2017-05-10T23:07:12Z

Agreed. @akfish, is my explanation of this PR’s value correct? Leaving aside the discussion of tests.

GeoffreyBooth · 2017-05-10T23:08:31Z

@jashkenas can you please reopen?

connec · 2017-05-11T10:46:43Z

Looks like the repo/branch has been removed so the PR can't be reopened.

akfish · 2017-05-11T11:11:57Z

Sorry for that. I should not have deleted the branch before the decision is final. I understand that the compiler usually don't mingle with webpack in this specific way thus there are some difficulties in communication. @GeoffreyBooth has put a lot of efforts into this and from his last comments I can see that we are on the same page.
I'll resend a PR.

GeoffreyBooth reviewed Apr 14, 2017

View reviewed changes

Use NOP moduleMain when generating parser with Jison

6cde4b5

Remove legacy debug code from browser.coffee

665eba1

Merge branch 'master' of github.com:jashkenas/coffeescript into fix/w…

ca93686

…ebpack

GeoffreyBooth requested changes Apr 18, 2017

View reviewed changes

GeoffreyBooth mentioned this pull request Apr 18, 2017

Module: requiring compiler for client-side use #4024

Closed

GeoffreyBooth mentioned this pull request Apr 19, 2017

Fix syntaxErrorToString helper #3911

Closed

jakesylvestre approved these changes Apr 19, 2017

View reviewed changes

Run browser tests against webpack build

e776d78

Fix newline at end of file

62a9315

jashkenas closed this May 10, 2017

jashkenas added change wontfix labels May 10, 2017

akfish deleted the fix/webpack branch May 10, 2017 21:21

GeoffreyBooth removed the wontfix label May 10, 2017

akfish mentioned this pull request May 11, 2017

[CS2] Module should be require-able in non-Node environments like Webpack and Browserify #4546

Merged

GeoffreyBooth changed the title ~~[WIP][CS2] Add webpack support~~ [CS2] Add webpack support Aug 3, 2017

GeoffreyBooth mentioned this pull request Feb 8, 2018

Consider transpiling /lib/ to ES5 for use as a library #4887

Closed

[CS2] Add webpack support #4501

[CS2] Add webpack support #4501

Conversation

akfish commented Apr 14, 2017

Changes

Tests

jashkenas commented Apr 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GeoffreyBooth commented Apr 14, 2017

akfish commented Apr 14, 2017

akfish commented Apr 14, 2017 • edited Loading

akfish commented Apr 14, 2017

jashkenas commented Apr 14, 2017

akfish commented Apr 14, 2017

GeoffreyBooth commented Apr 14, 2017

akfish commented Apr 14, 2017

lydell commented Apr 15, 2017

GeoffreyBooth commented Apr 15, 2017

akfish commented Apr 15, 2017

akfish commented Apr 15, 2017 • edited Loading

akfish commented Apr 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akfish Apr 18, 2017 • 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

GeoffreyBooth commented Apr 18, 2017

akfish commented Apr 18, 2017

GeoffreyBooth commented Apr 18, 2017

akfish commented Apr 18, 2017 • edited Loading

GeoffreyBooth commented Apr 19, 2017

akfish commented Apr 19, 2017

GeoffreyBooth commented Apr 19, 2017

GeoffreyBooth commented May 10, 2017

akfish commented May 10, 2017

akfish commented May 10, 2017

akfish commented May 10, 2017

GeoffreyBooth commented May 10, 2017

akfish commented May 10, 2017

GeoffreyBooth commented May 10, 2017

akfish commented May 10, 2017

GeoffreyBooth commented May 10, 2017

akfish commented May 10, 2017

jashkenas commented May 10, 2017

akfish commented May 10, 2017

Radivarig commented May 10, 2017

GeoffreyBooth commented May 10, 2017 • edited Loading

jashkenas commented May 10, 2017

carlsmith commented May 10, 2017

GeoffreyBooth commented May 10, 2017

GeoffreyBooth commented May 10, 2017

connec commented May 11, 2017

akfish commented May 11, 2017 • edited Loading

akfish commented Apr 14, 2017 •

edited

Loading

akfish commented Apr 15, 2017 •

edited

Loading

akfish commented Apr 15, 2017 •

edited

Loading

akfish Apr 18, 2017 •

edited

Loading

akfish commented Apr 18, 2017 •

edited

Loading

GeoffreyBooth commented May 10, 2017 •

edited

Loading

akfish commented May 11, 2017 •

edited

Loading