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

perf: don't use middleware if has build #1148

Merged
merged 9 commits into from
Jul 9, 2020

Conversation

mrloop
Copy link
Contributor

@mrloop mrloop commented May 1, 2020

This PR depends on a PR for ember-cli being merged first, see ember-cli/ember-cli#9205 merged

ember serve and ember test has a path flag to specify
an existing ember build to use, without rebuilding. ember-cli-typescript
will add typechecking middleware regardless of ember using an existing
build, the typechecking middleware is not required. This can slow
down the initial request to express server significantly.

For instance one project spends 49 secs for initial typecheck on first
request.

$ DEBUG=*:* ember s --path dist
...
– Serving on http://localhost:4200/admin/
...
ember-cli-typescript:typecheck-worker Typecheck complete (0 diagnostics) +49s
...

@jamescdavis
Copy link
Member

I think this could be a nice perf improvement. I've experienced the same and agree that it doesn't make sense to run the type-checking middleware when you're just using an existing build.

@rwjblue
Copy link
Contributor

rwjblue commented May 11, 2020

FWIW, the ember-cli PR should land into the ember-cli@3.19.0-beta cycle today/tomorrow.

@mrloop
Copy link
Contributor Author

mrloop commented May 11, 2020

Thanks @rwjblue I'll update this PR, add a test and remove the prettier update and not use optional chaining, this can be added in another PR.

@mrloop
Copy link
Contributor Author

mrloop commented May 14, 2020

I've updated the PR ready for review, removing the prettier update and optional chaining causing prettier linting to fail. I've added tests to check that type checking worker is running or not running for both ember serve and ember test depending on the --path flag.

One of the required checks are failing CI / Test (linux, floating dependencies) (pull_request), this looks unrelated to PR and not sure how to fix it.

@chriskrycho
Copy link
Member

Thank you! One of us will try to get to this in the next couple days!

`ember serve` and `ember test` has a `path` flag to specify
an existing ember build to use, without rebuilding. `ember-cli-typescript`
will add typechecking middleware regardless of ember using an existing
build, the typechecking middleware is not required. This can slow
down the initial request to express server significantly.

For instance one of my projects spends 49 secs for initial typecheck on
first request.

```sh
$ DEBUG=*:* ember s --path dist
...
– Serving on http://localhost:4200/admin/
...
ember-cli-typescript:typecheck-worker Typecheck complete (0 diagnostics) +49s
...
```
@mrloop
Copy link
Contributor Author

mrloop commented Jun 12, 2020

Rebased on master

@mrloop
Copy link
Contributor Author

mrloop commented Jun 25, 2020

@chriskrycho is there anything I can do to help get this over line and merged? Is the testing approach good or is there a better way of doing it?

@jamescdavis jamescdavis added this to the v4.0 milestone Jul 2, 2020
mrloop and others added 2 commits July 3, 2020 16:50
test: suggested updates for testing if typecheck middleware is installed
@jamescdavis jamescdavis merged commit 3f86356 into typed-ember:master Jul 9, 2020
@jamescdavis
Copy link
Member

Released in 4.0.0-rc.1

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.

5 participants