-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add ES6 Import Validation #209
Conversation
i think the The rest seems awesome. |
whitelist: { | ||
'ember/resolver': ['default'], | ||
'ember-qunit': [ | ||
'globalize', |
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.
it would be cool if some conventions for vendor'd addons could exist to learn these import/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.
I think this is a side effect of mixing your module loader systems. ember-qunit
is not really an ES6 module when the page loads, but it's imported like one and your sort of magically depending on the fact that when it's all transpiled to amd that import is becoming a require('ember-qunit')
call.
You could maybe do a relative import path to the actual vendor file in '../vendor/ember-qunit/lib/main'
but then your import from 'ember'
is gonna fail so you have to do it all the way down.
At Sprout we handle vendor files that are not ES6 modules a little differently, we basically rely on them being on the page before hand (usually concatenated into a single vendors.js file) and then write an ES6 shimming vendor/libraries file:
/* File: script/vendor/libraries.js */
// Just in case something wants to pass in a custom globals val
var globals = globals || window;
export var jQuery = globals.jQuery;
export var $ = globals.jQuery;
export var underscore = globals._;
export var lodash = globals._;
export var _ = globals._;
export var Backbone = globals.Backbone;
export var icanhaz = globals.ich;
export var ich = globals.ich;
export var cconsole = globals.cconsole;
export var Modernizr = globals.Modernizr;
export var WallTime = globals.WallTime;
export var TwitterCldr = globals.TwitterCldr;
export var twttr = globals.twttr;
export var Hogan = globals.Hogan;
Then from other files we import with:
import { _, Backbone } from 'vendor/libraries';
Of course, you could always write individual shims per library sort of like requirejs makes it easy to do. At any rate, this lets us have a single flexibility point and keep our module "philosophy" coherent and staying in ES6 land. I think it would be nice to have a native shimming capability in ES6, I might bring it up on esdiscuss.
this looks great, when I'm back in NYC tomorrow I plan to give a whirl. Do you think you can rebase + update accordingly? |
- Add dependency broccoli-es6-import-validate - Add validateJs tree (only when env !== 'production')
Updated
|
rerunning the failed test |
@jgable is this good to go? |
I believe so. Tested out with same test cases as before and they seemed to work. |
@jgable awesome, I'll merge it in after work :) Thanks man! |
just tried it out, its pretty good. :) |
I'm still seeing the I'm starting off with just a simple route test. The module prefix is import TodosRoute from 'ember-cli-todomvc/routes/todos';
// ... Running
I've tried a few variations of the import statement path but each one gives the same error. Any idea what I could be doing wrong? |
@javierjulio that validation only works for properties missing on existing modules. The error you see is broccoli telling you the entire module is missing from the tree. Currently test + app are separate trees, I believe the goal will be make an app tree, and a app + test tree. Which should mitigate this issue (cc @rjackson ) |
@javierjulio see #274 for more information on your issue. There is also a reference to a Brocfile that may serve as a temporary workaround for you. |
Update fs-extra to version 0.28.0 🚀 Hello 👋 🚀🚀🚀 [fs-extra](https://www.npmjs.com/package/fs-extra) just published its new version 0.28.0, which **is not covered by your current version range**. If this pull request passes your tests you can publish your software with the latest version of fs-extra – otherwise use this branch to work on adaptions and fixes. Happy fixing and merging 🌴 --- The new version differs by 13 commits . - [`ef35b70`](jprichardson/node-fs-extra@ef35b70) `0.28.0` - [`8c56e5b`](jprichardson/node-fs-extra@8c56e5b) `(Closes #209, Closes #237) lib/mkdirs: if invalid path char, return callback OR throw err` - [`a0cb04b`](jprichardson/node-fs-extra@a0cb04b) `(Closes #93) libs/mkdirs: prevent stack overflow if drive is not mounted in Windows` - [`76dcfa9`](jprichardson/node-fs-extra@76dcfa9) `lib/mkdirs/tests/root: skip if network drive` - [`f5c64d5`](jprichardson/node-fs-extra@f5c64d5) `gitignore: add npm debug` - [`5747fb3`](jprichardson/node-fs-extra@5747fb3) `(Closes #192) removed createOutputStream()` - [`2a5e355`](jprichardson/node-fs-extra@2a5e355) `readme: update note about hacking on fs-extra` - [`5652b96`](jprichardson/node-fs-extra@5652b96) `changelog: update issue links (auto generated)` - [`e2a7dae`](jprichardson/node-fs-extra@e2a7dae) `appveyor: node v5` - [`e8d2949`](jprichardson/node-fs-extra@e8d2949) `package: update travis` - [`a524434`](jprichardson/node-fs-extra@a524434) `readme: fix date of note` - [`e21cc16`](jprichardson/node-fs-extra@e21cc16) `readme: note about dropping old Node.js` - [`ad98995`](jprichardson/node-fs-extra@ad98995) `readme: update` See the [full diff](jprichardson/node-fs-extra@6fb9fc7...ef35b70). --- This pull request was created by [greenkeeper.io](http://greenkeeper.io/). It keeps your software up to date, all the time. <sub> Tired of seeing this sponsor message? Upgrade to the supporter plan! You'll also get your pull requests faster ⚡ </sub>
Update fs-extra to version 0.28.0 🚀 Hello 👋 🚀🚀🚀 [fs-extra](https://www.npmjs.com/package/fs-extra) just published its new version 0.28.0, which **is not covered by your current version range**. If this pull request passes your tests you can publish your software with the latest version of fs-extra – otherwise use this branch to work on adaptions and fixes. Happy fixing and merging 🌴 --- The new version differs by 13 commits . - [`ef35b70`](jprichardson/node-fs-extra@ef35b70) `0.28.0` - [`8c56e5b`](jprichardson/node-fs-extra@8c56e5b) `(Closes #209, Closes #237) lib/mkdirs: if invalid path char, return callback OR throw err` - [`a0cb04b`](jprichardson/node-fs-extra@a0cb04b) `(Closes #93) libs/mkdirs: prevent stack overflow if drive is not mounted in Windows` - [`76dcfa9`](jprichardson/node-fs-extra@76dcfa9) `lib/mkdirs/tests/root: skip if network drive` - [`f5c64d5`](jprichardson/node-fs-extra@f5c64d5) `gitignore: add npm debug` - [`5747fb3`](jprichardson/node-fs-extra@5747fb3) `(Closes #192) removed createOutputStream()` - [`2a5e355`](jprichardson/node-fs-extra@2a5e355) `readme: update note about hacking on fs-extra` - [`5652b96`](jprichardson/node-fs-extra@5652b96) `changelog: update issue links (auto generated)` - [`e2a7dae`](jprichardson/node-fs-extra@e2a7dae) `appveyor: node v5` - [`e8d2949`](jprichardson/node-fs-extra@e8d2949) `package: update travis` - [`a524434`](jprichardson/node-fs-extra@a524434) `readme: fix date of note` - [`e21cc16`](jprichardson/node-fs-extra@e21cc16) `readme: note about dropping old Node.js` - [`ad98995`](jprichardson/node-fs-extra@ad98995) `readme: update` See the [full diff](jprichardson/node-fs-extra@6fb9fc7...ef35b70). --- This pull request was created by [greenkeeper.io](http://greenkeeper.io/). It keeps your software up to date, all the time. <sub> Tired of seeing this sponsor message? Upgrade to the supporter plan! You'll also get your pull requests faster ⚡ </sub>
Progress on #36
Wanted to get this up for review. I've tested by running locally with
npm link
, creating a new project withember new ember-test
thenember build
andember server
in the new project.I tried to test out importing from a non existent module like
import Blah from 'ember-test/something/missing'
but something is failing with an ENOENT because that file doesn't exist that is being imported from. But, when I tested by adding an extra named import likeI get a proper warning from the validationJs tree.