meteor / todos

The example app "Todos", written following the Meteor Guide
Other
535 stars 367 forks source link

Avoid unnecessary server rebuilds #159

Closed detrohutt closed 8 years ago

detrohutt commented 8 years ago

...by renaming the 'imports/ui' folder to 'imports/client' and updating import references.

Before these changes, editing a file in the 'imports/ui' folder causes a server rebuild. After these changes, editing a file in the 'imports/client' folder only causes a client refresh.

Update: I noticed the circleci test doesn't run the --full-app tests.. I'm unable to get the tests working at all locally, even on the master or react branches, so I can't confirm that these changes don't break the tests. This issue leads me to believe that's a possibility.

What would be the correct course of action for me at this point(I'm pretty new to making PRs)? Should I just close the PR or leave it open to see if someone else can confirm whether or not the tests work?

apollo-cla commented 8 years ago

@detrohutt: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

hwillson commented 8 years ago

Thanks for the PR @detrohutt. I think for now we'll have to hold off on merging this as the issue that using imports/client works around should really be addressed. Issue meteor/meteor/issues/7434 has been created to track this. Hopefully the 1.4.2 release (meteor/meteor/pull/7668) will help with this. If it doesn't then we'll have to update both the Guide and todos app to reflect these changes. We can either keep this PR open as a reminder, or close it and revisit after the 1.4.2 release - whichever you prefer. Thanks again!

detrohutt commented 8 years ago

That makes sense. I'm excited about the 1.4.2 release! I'll close this for now and I'll check how relevant this is after 1.4.2 releases officially. If it still makes a big difference I'll reopen for consideration. Thanks for taking a look!