[BUGFIX] Ensure Ember build is complete. #349
Conversation
Checks to see if index.html file exists for each app before declaring the build complete. Fixes thoughtbot#331.
| @@ -32,7 +32,7 @@ def wait! | |||
| attr_reader :name, :paths | |||
|
|
|||
| def complete? | |||
| !paths.lockfile.exist? | |||
| !paths.lockfile.exist? && paths.dist.join("index.html").exist? | |||
There was a problem hiding this comment.
Does this fix the bug you've been seeing?
The addon creates the lockfile when the build starts, then removes it when it's finished.
If the lockfile no longer exists, I'm not sure what good waiting on the index.html file will do.
It's likely that it'll hang indefinitely
If this DOES fix your issue, I'm curious what would make the index.html file suddenly materialize, outside the addon-managed build cycle
There was a problem hiding this comment.
Yes, it does fix my issue.
I couldn't find where the lockfile gets removed... I looked but no luck. More correct behavior would be to wait until index.html exists in dist before the lockfile gets removed.
Overall, expected behavior is that index.html should exist before declaring build done though, right? Open to suggestions on where to ensure that logic.
There was a problem hiding this comment.
@patcoll the lockfile is removed by https://github.com/rondale-sc/ember-cli-rails-addon/blob/cadb98ad22462664fda501fb7213649123fd4c63/index.js#L63-L65.
I agree, the build should block until index.html exists.
There was a problem hiding this comment.
@patcoll one thing I couldn't clarify before the end of our session:
Are you using rack-timeout or have any other request timeout strategies?
There was a problem hiding this comment.
Yes, I do. My rack-timeout logic looks like this (in a Rails initializer):
if Rails.env.development? or Rails.env.test?
Rack::Timeout.timeout = 60
else
Rack::Timeout.timeout = 15 # seconds
end
With the newest non-Sprockets builds it never reaches anywhere near 15 seconds of initial load time. It's more like 5 seconds on average right now.
There was a problem hiding this comment.
Hm, interesting. Could be a fsExtra.copySync bug or similar.
There was a problem hiding this comment.
The addon no longer copies the index.html file:
rondale-sc/ember-cli-rails-addon@fd6d6ba
Copying the index was a means of integrating with Sprockets, and is no longer necessary.
|
I'll use these two versions and see if that removal of fs-extra fixes my issue: |
|
Nope. It took 5-6 tries to get error #331, but I'm still getting it. Same one as in the gist. |
|
Definitely seems like a race condition of some sort. |
Closes [#331]. Closes [#349]. According to the [addon hook documentation][hooks]: `#postBuild` > Gives access to the result of the tree, and the location of the output. `#outputReady` > Hook called after the build has been processed and the files have been > copied to the output directory Since we're now blocking until the `index.html` it built and in the output directory, `outputReady` is the behavior we depend on. [hooks]: https://github.com/ember-cli/ember-cli/blob/082d559757b3d6d186fc1c3cd1b7c2bb1ad10b3f/ADDON_HOOKS.md#outputready [#331]: thoughtbot/ember-cli-rails#331 [#349]: thoughtbot/ember-cli-rails#349
|
@patcoll Could you please try out rondale-sc/ember-cli-rails-addon#27? |
|
That totally works. You're a savant. I'm closing this. |
Checks to see if index.html file exists for each app before declaring the build complete. Fixes #331.