Skip to content
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

Remove emberjs-build #15037

Merged
merged 1 commit into from
Mar 21, 2017
Merged

Remove emberjs-build #15037

merged 1 commit into from
Mar 21, 2017

Conversation

chadhietala
Copy link
Contributor

@chadhietala chadhietala commented Mar 18, 2017

This removes emberjs-build and introduces babel @ 6.0.0. Also sets things up Svelte and RFC#176.

Also this makes rebuilds about twice as fast.

screen shot 2017-03-18 at 12 19 26 pm

@chadhietala chadhietala force-pushed the yaks-on-yaks branch 4 times, most recently from e8cefba to e34f936 Compare March 20, 2017 18:44
@chadhietala chadhietala changed the title [WIP] Remove emberjs-build Remove emberjs-build Mar 20, 2017
package.json Outdated
"babel-plugin-transform-es2015-computed-properties": "^6.22.0",
"babel-plugin-transform-es2015-destructuring": "^6.23.0",
"babel-plugin-transform-es2015-modules-amd": "^6.24.0",
"babel-plugin-transform-es2015-modules-commonjs": "^6.24.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed

var tree = addon.treeFor('addon');

return new Funnel(tree, {
srcDir: 'modules'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes in ember-cli@2.13. We no longer emit treeForAddon into modules/.


module.exports = function emberTestES(name) {
return new Funnel(`packages/${name}/tests`, {
getDestinationPath(relativePath) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be changed to avoid requiring us to stat and symlink all the files:

return new Funnel(`packages/${name}/tests`, {
  destDir: `${name}/tests/`
});

function getLibPath(packagePath) {
let packageJson = require(packagePath);

return path.dirname(packageJson['module'] || packageJson['main'] || '.');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last || '.' seems odd, do we need that?

GlimmerTemplatePrecompiler.prototype.processString = function(content, relativePath) {
var compiled = this.precompile(content, { meta: { moduleName: relativePath } });
var template = 'import template from "../template";\n';
template += 'export default template(' + compiled + ');';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could probably use template strings nicely here:

const stripIndent = require('common-tags').stripIndent;
//...snip...
return stripIndent`
  import template from '../template';
  export default template(${compiled});
`;

}

injectNodeGlobals.baseDir = function() {
return 'babel-plugin-transform-es2015-modules-amd';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd, it should return a path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it's going to do hash-for-dep. So __dirname is no good, this thing doesn't have any dependencies and deriving the cachkey from Ember's deps seems wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does, it is dependent on babel and its own file, but it at least should be returning itself.

emberFeaturesES,
packageManagerJSONs,
nodeTests
} = require('./broccoli/packages');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Destructuring requires node 6 😢. I'm ok with requiring node@6 here, but when we ultimately have ember-cli apps building ember-source themselves (when not running a tagged version) we will need to downgrade to support node@4.

@@ -7,7 +7,7 @@ module.exports = function(features) {
'ember-metal': { trees: null, requirements: ['ember-environment', 'ember-utils'], vendorRequirements: ['backburner'] },
'ember-debug': { trees: null, requirements: [] },
'ember-runtime': { trees: null, vendorRequirements: ['rsvp'], requirements: ['container', 'ember-environment', 'ember-console', 'ember-metal'] },
'ember-views': { trees: null, requirements: ['ember-runtime'] },
'ember-views': { trees: null, requirements: ['ember-runtime'], skipTests: true },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there no tests, or did you move them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no more tests in there.

@@ -64,7 +64,7 @@ function K() {}

function ensureRunloop() {
if (!run) {
run = require('ember-metal/run_loop').default;
run = require('ember-metal').run;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to bust a cycle?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely

if (protoProps) defineProperties(Constructor.prototype, protoProps);
if (staticProps) defineProperties(Constructor, staticProps);
return Constructor;
}

function interopExportWildcard(obj, defaults) {
export function interopExportWildcard(obj, defaults) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think with noInterop and strict set for modules, this should not be needed?

This also rolls up ember-metal
@rwjblue rwjblue merged commit 0947407 into emberjs:master Mar 21, 2017
@chadhietala chadhietala deleted the yaks-on-yaks branch March 22, 2017 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants