-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CS2] Module should be require-able in non-Node environments like Webpack and Browserify #4546
Conversation
* 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
# Conflicts: # Cakefile # bower.json # package.json # src/browser.coffee # src/cake.coffee # src/coffeescript.coffee # src/command.coffee # src/index.coffee # src/register.coffee # src/repl.coffee
… for the lack of stub when running the browser tests in Node
# Conflicts: # docs/v2/browser-compiler/coffeescript.js # lib/coffeescript/coffeescript.js
Cakefile
Outdated
task 'build:webpack', 'build webpack test bundle', -> | ||
args = [ | ||
"./node_modules/webpack/bin/webpack.js" | ||
"--entry=./test/webpack/entry.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this entry, and not browser.js as above?
Cakefile
Outdated
|
||
task 'test:webpack', 'run test suite against the webpack test bundle', -> | ||
loader = """ | ||
var __exports__ = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this variable? Is it distinct from CommonJS exports
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Webpack will scope known globals. It is not possible to access them directly. This, along with the custom entry file and the loader script in test:webpack task, allows the bundled CoffeeScript to escape thus it can be required in node environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be a better way to do this. I'll get back to you later.
Turns out there's an webpack option for that and no additional configuration files nor plugins required. That makes more sense. Don't know what I was thinking back then. |
@lydell or @jashkenas did you have any further notes? I’m leaving the Somehow |
Looks great! |
…pack and Browserify (#4546) * Add webpack support * 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 * Use NOP moduleMain when generating parser with Jison * Remove legacy debug code from browser.coffee * Improve comments, style * Fix path * Remove stub that was only to avoid breaking browser tests; compensate for the lack of stub when running the browser tests in Node * Update output * Add test:webpack task to Cakefile * Update output files * Run browser tests against webpack build * Fix newline at end of file * Export webpack test bundle as CommonJS module * Remove build:webpack task * Save webpack build to tmpdir; suppress build output unless it fails
Resend #4501