Skip to content

Initial MU support #199

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

Merged
merged 4 commits into from
Jul 25, 2018
Merged

Initial MU support #199

merged 4 commits into from
Jul 25, 2018

Conversation

dfreeman
Copy link
Member

@dfreeman dfreeman commented Apr 27, 2018

Fixes #198. Note that there will almost certainly be follow-on work as things evolve, particularly in the addon space. If we're lucky, Babel 7 will land soon and we'll have a lot less to worry about 😉

This PR includes:

  • Build support for MU app and addon projects
  • ember g ember-cli-typescript support for app and addon projects

It does not include:

  • Blueprint updates for MU in-repo addons
  • Any MU blueprints for runtime entities (routes, components, etc)
  • Out-of-the-box tsconfig support for import foo from 'addon-root'; (not actually clear whether/how that works with MU)

The vast majority of the changes here are updates to the default blueprint and the addition of test coverage for MU scenarios. The meat of the change, in the incremental compiler and ts:precompile command, only constitutes a couple dozen lines of diff total.

Given that, I believe this is a pretty low-risk change, and I think it could be nice to try and include in a beta before 1.3.0 is stable, but if we'd rather hold off I can see the argument for that. @chriskrycho @dwickern @jamescdavis thoughts?

@dfreeman
Copy link
Member Author

Tests are exploding on master, too — looks like we're getting type conflicts with ember-try.

@@ -32,7 +32,10 @@ module.exports = {
let inRepoAddons = (this.project.pkg['ember-addon'] || {}).paths || [];
let hasMirage = 'ember-cli-mirage' in (this.project.pkg.devDependencies || {});
let isAddon = this.project.isEmberCLIAddon();
let includes = ['app', isAddon && 'addon', 'tests', 'types'].concat(inRepoAddons).filter(Boolean);
let isMU = this._detectMU();
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli Apr 27, 2018

Choose a reason for hiding this comment

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

does javascript support unicode for variables? isμ? :trollface:

I actually just looked up upper case mu/μ, and it's pretty boring. (capital M). lame.

@NullVoxPopuli
Copy link
Collaborator

NullVoxPopuli commented Apr 27, 2018

I've been testing out this branch, couple things I've noticed (Not sure if pre-existing or not)

I started with deleting tmp and dist, just in case :-)
everytime I ran in to an issue, I killed everything and started from scratch.

  • upon rename of file from js -> ts, the ember server doesn't not re-process the files. it detects the changes, but freezes. only killable via kill -9 (ctrl+c / kill aren't good enough)
  • using Object.keys(window.requirejs.entries).filter(b => b.includes("service")), my typescript service isn't detected (located at src/services/relay-connection) (none of my typescript files show up in the list)
  • saving a typescript file with no changes causes the same behavior as the js -> ts rename

though, it could also be that the master version of ember-cli is broken.
from yarn.lock:

"ember-cli@github:ember-cli/ember-cli":
  resolved "https://codeload.github.com/ember-cli/ember-cli/tar.gz/d16993716d6d3062c977b12d28a3cef27de9d9e2"

"ember-cli-typescript@github:typed-ember/ember-cli-typescript#mu":
  resolved "https://codeload.github.com/typed-ember/ember-cli-typescript/tar.gz/697d768d86c14ebd4be8d17b1a53bd91644557c4"

@dfreeman
Copy link
Member Author

@NullVoxPopuli Here's the app I was testing this all out with: dfreeman/typescript-mu-app@9b78ad6...master — I haven't been able to reproduce any of what you're describing with that, so I'm curious whether the difference is in the project or in the environment.

I did push one additional commit to the mu branch to filter out an in-repo addon we use for testing, but I wouldn't expect that to have been causing the issues you're seeing 🤔

@NullVoxPopuli
Copy link
Collaborator

@dfreeman interesting. I'm using this app: https://github.com/NullVoxPopuli/emberclear

Your diff was pretty helpful. I managed to find some differences between our apps, and made some updates here: NullVoxPopuli/emberclear@f3c8172

everything seems to work now e-c-ts-wise :-)

Thanks a ton!!

@dfreeman
Copy link
Member Author

Glad to hear that solved it! Looking at your diff, I suspect src/* in your include array was causing the issue.

@chriskrycho chriskrycho added this to the Module Unification milestone Apr 30, 2018
@NullVoxPopuli
Copy link
Collaborator

been using this for a while. still good!
anything preventing merging?

@NullVoxPopuli
Copy link
Collaborator

Actually, I'm getting an infinite tsc loop of errors printing to the console now. hmm

@dfreeman
Copy link
Member Author

dfreeman commented Jun 4, 2018

@NullVoxPopuli rebased off master, which should make things happy with 2.9 now

@NullVoxPopuli
Copy link
Collaborator

Guess we need a src/ui/styles directory now?

yeah, this was discovered in the #st-module-unification channel on slack yesterday or Saturday, I think.

@NullVoxPopuli rebased off master, which should make things happy with 2.9 now

Thanks, @dfreeman !! you are the best!!!
What I did to get my app running using the new mu branch updates: NullVoxPopuli/emberclear@a1945cb

Also, I should probably follow master as well for bug fixes and such so I can test things more thoroughly.

@chriskrycho chriskrycho added build Ideas for or bugs with the build process enhancement labels Jul 6, 2018
@dfreeman dfreeman merged commit 5c468af into master Jul 25, 2018
@dfreeman dfreeman deleted the mu branch July 25, 2018 23:03
@NullVoxPopuli
Copy link
Collaborator

Alt Text

@dfreeman dfreeman restored the mu branch July 25, 2018 23:34
@dfreeman
Copy link
Member Author

Just occurred to me I should leave this branch around for a while until we have a release out and anyone relying on it can switch over... 😬

@jamescdavis
Copy link
Member

@dfreeman should we delete mu branch now that this is in a release?

@dfreeman dfreeman deleted the mu branch August 9, 2018 17:52
@championswimmer
Copy link

will support for routes, components etc be added to in future ?

@dfreeman
Copy link
Member Author

dfreeman commented Nov 5, 2018

@championswimmer if you're talking about ember generate, there's a quest issue for that over on https://github.com/typed-ember/ember-cli-typescript-blueprints/issues/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Ideas for or bugs with the build process enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants