[Feature] Update initializer blueprint for module unification by sduquej · Pull Request #16489 · emberjs/ember.js

sduquej · 2018-04-11T18:26:37Z

TODO:

Addons – unsure of how this might work given the following in RFC143

The specific format of collection and type declarations for addons is TBD.

GavinJoyce · 2018-04-16T07:22:46Z

node-tests/blueprints/initializer-test.js

+        );
+
+        expect(_file('src/init/initializers/foo/bar-test.js')).to.contain(
+          "import { initialize } from 'my-app/initializers/foo/bar';"


I think this will have to change to point to the new path under src

I came to this PR looking for how to do that, haha. I'm in the same situation where my tests are passing without updating the import statement and I'm not sure why 🤔 Or where the import statement is generated.

I think I've got it sorted now 👍

GavinJoyce · 2018-04-16T07:24:05Z

Perhaps update the title to be Update initializer blueprint for module unification?

GavinJoyce · 2018-04-16T07:28:49Z

We'll also need to add tests to initializer-test-test.js to ensure that we're generating co-located tests.

I manually tried this in a local MU app and it seems to fail with a Uncaught Error: mu-app/src/init/initializers/foo-test must export an initializer. exception. We might have to modify https://github.com/ember-cli/ember-load-initializers to ignore colocated tests:

LMK if you'd like to pair on this

GavinJoyce · 2018-04-16T07:31:50Z

The specific format of collection and type declarations for addons is TBD

For the purpose of making progress in the PR, I think a good assumption is that they should follow the same rules as apps. If we hit any barriers when implementing, we can raise it.

GavinJoyce · 2018-04-20T08:34:00Z

@sduquej ^ that's been resolved, perhaps you could rebase?

Also, could you add an in-repo-addon test similar to this? This will ensure that we're not breaking existing functionality.

sduquej · 2018-04-21T11:23:15Z

👍 I've rebased.
The initializer blueprint already had tests similar to those.
Thanks for unblocking this! 🙌

sduquej · 2018-06-03T23:28:19Z

node-tests/fixtures/initializer-test/module-unification/default.js

+import Application from '@ember/application';
+import { run } from '@ember/runloop';
+
+import { initialize } from 'my-app/init/initializers/foo';


the addition of init/ is the only new thing in these fixture files.

There's a possibility that they can get out of date if changes to the classic fixtures happened. While not ideal the risk seems minimum given how infrequent these type of changes are and duplicating these has the upside of easier clean-up and similarity to what was done in ember-cli

GavinJoyce · 2018-06-05T11:07:17Z

blueprints/initializer-test/index.js

+          if (options.pod) {
+            throw 'Pods arenʼt supported within a module unification app';
+          } else if (options.inDummy) {
+            return path.join('tests', 'dummy', 'src/init');


return path.join('tests', 'dummy', 'src', 'init'); perhaps?

GavinJoyce · 2018-06-05T11:07:43Z

blueprints/initializer-test/index.js

+          } else if (options.inDummy) {
+            return path.join('tests', 'dummy', 'src/init');
+          }
+          return 'src/init';


return path.join('src', 'init'); perhaps?

GavinJoyce · 2018-06-05T11:09:52Z

blueprints/initializer-test/mocha-files/__root__/__testType__/__path__/__name__-test.js

 import { run } from '@ember/runloop';
 import Application from '@ember/application';
-import { initialize } from '<%= dasherizedModulePrefix %>/initializers/<%= dasherizedModuleName %>';
+import { initialize } from '<%= dasherizedModulePrefix %>/<% if (isModuleUnificationProject) { %>init/<% } %>initializers/<%= dasherizedModuleName %>';


perhaps it would be simpler to replace dasherizedModulePrefix with a modulePrefix in locals so that we don't have to litter lots of files with <% if (isModuleUnificationProject) { %>init/<% } %> statements?

👍 great suggestion!

sduquej · 2018-06-07T18:28:19Z

@GavinJoyce I think this is ready for review whenever you've some time 👀

GavinJoyce · 2018-06-07T18:30:02Z

@sduquej 👀. Can you prefix the commit with [Feature]?

rwjblue · 2018-06-08T13:34:07Z

Restarted CI

sduquej · 2018-06-08T14:25:36Z

@rwjblue could you give it one more kick please? seems to have timed out

GavinJoyce

I'm 👍 too, nice one @sduquej

rwjblue · 2018-06-08T17:28:00Z

Awesome, thank you!

GavinJoyce reviewed Apr 16, 2018

View reviewed changes

GavinJoyce mentioned this pull request Apr 16, 2018

[WIP] [Feature] instance-initializer blueprint for module unification #16491

Closed

3 tasks

sduquej changed the title ~~[WIP] [Feature] Update service blueprint for module unification~~ [WIP] [Feature] Update initializer blueprint for module unification Apr 16, 2018

GavinJoyce mentioned this pull request Apr 16, 2018

allow colocated test files ember-cli/ember-load-initializers#53

Merged

GavinJoyce mentioned this pull request Apr 21, 2018

Module Unification: Blueprints ember-cli/ember-cli#7530

Closed

29 tasks

locks added the Needs Submitter Response label Apr 21, 2018

sduquej force-pushed the sd/initializer-blueprint branch from 93ad120 to 916981f Compare April 21, 2018 11:00

sduquej force-pushed the sd/initializer-blueprint branch 2 times, most recently from 03a4396 to 98c2e32 Compare June 3, 2018 23:20

sduquej commented Jun 3, 2018

View reviewed changes

sduquej force-pushed the sd/initializer-blueprint branch from 98c2e32 to f3cd9ff Compare June 3, 2018 23:29

sduquej changed the title ~~[WIP] [Feature] Update initializer blueprint for module unification~~ [Feature] Update initializer blueprint for module unification Jun 4, 2018

GavinJoyce reviewed Jun 5, 2018

View reviewed changes

sduquej force-pushed the sd/initializer-blueprint branch from f3cd9ff to a15ca10 Compare June 6, 2018 20:43

sduquej force-pushed the sd/initializer-blueprint branch 5 times, most recently from 7ecc071 to fc1014c Compare June 8, 2018 12:14

[Feature] Update initializer blueprint for module unification

3661007

sduquej force-pushed the sd/initializer-blueprint branch from fc1014c to 3661007 Compare June 8, 2018 12:21

rwjblue approved these changes Jun 8, 2018

View reviewed changes

GavinJoyce approved these changes Jun 8, 2018

View reviewed changes

rwjblue merged commit bd8ad77 into emberjs:master Jun 8, 2018

sduquej deleted the sd/initializer-blueprint branch June 9, 2018 06:55

Uh oh!

Conversation

sduquej commented Apr 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GavinJoyce Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lbeauvilliers May 1, 2018

Choose a reason for hiding this comment

Uh oh!

lbeauvilliers May 1, 2018

Choose a reason for hiding this comment

Uh oh!

GavinJoyce commented Apr 16, 2018

Uh oh!

GavinJoyce commented Apr 16, 2018

Uh oh!

GavinJoyce commented Apr 16, 2018

Uh oh!

GavinJoyce commented Apr 20, 2018

Uh oh!

sduquej commented Apr 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sduquej Jun 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GavinJoyce Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

GavinJoyce Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

GavinJoyce Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sduquej Jun 5, 2018

Choose a reason for hiding this comment

Uh oh!

sduquej commented Jun 7, 2018

Uh oh!

GavinJoyce commented Jun 7, 2018

Uh oh!

rwjblue commented Jun 8, 2018

Uh oh!

sduquej commented Jun 8, 2018

Uh oh!

GavinJoyce left a comment

Choose a reason for hiding this comment

Uh oh!

rwjblue commented Jun 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sduquej commented Apr 11, 2018 •

edited

Loading

GavinJoyce Apr 16, 2018 •

edited

Loading

sduquej commented Apr 21, 2018 •

edited

Loading

sduquej Jun 3, 2018 •

edited

Loading

GavinJoyce Jun 5, 2018 •

edited

Loading