Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Changed Autoload to use Module#Autoload in AS::Dependencies #6

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

tsun1215
Copy link

@tsun1215 tsun1215 commented May 2, 2015

(This is in response to: https://gist.github.com/matthewd/9e54b38bc5184134388b)

Summary

The autoload functionality of ActiveSupport::Dependencies has been changed from using Object.const_missing to Object.autoload for pre-installing autoloads for all constants to be loaded.

This behavior is achieved for all types of constants, including nested multi-file ones, by creating temporary files (using the Tempfile gem) that serve as intermediate files for setting up and loading constants. Currently, single file constants are loaded using a temporary file so that we can decide to hook in to the autoloading of a constant if necessary--which may be the case when trying to add the new constant watcher.

Example

In the test fixture, we have the folder structure: https://github.com/rails/rails/tree/master/activesupport/test/autoloading_fixtures/a

Our autoload will create the following structure in a temporary file:

module A
      module C
        autoload :D, "/projects/rails/activesupport/test/autoloading_fixtures/a/c/d.rb"
        module E
          autoload :F, "/projects/rails/activesupport/test/autoloading_fixtures/a/c/e/f.rb"
        end
      end
      autoload :B, "/projects/rails/activesupport/test/autoloading_fixtures/a/b.rb"
    end

Then :A is autoloaded into Object, pointing to this file.

Compatibility

Currently, 17 of the activesupport/test/dependencies_test.rb are marked as skipped. Many have the comment NOT SUPPORTED indicating that it is a test that tests deprecated behavior (for example, using load_missing_constant); these have been left there for review and examples. Other tests were either not gotten to or specify a functionality that is not yet implemented.

In the rest of the ActiveSupport suite, the changes break 1 test in MessageVerifier (it seems like it tests an old behavior) and 2 tests in DescendantsTracker.

Unfinished

  • Using WatchStack to determine new constants that are loaded when a tempfile is autoloaded

@tsun1215
Copy link
Author

tsun1215 commented May 2, 2015

We will merge the commits into one after the changes are discussed.

@rafaelfranca
Copy link

This means that temporary files inside app/model or related directories will be created?

@rafaelfranca
Copy link

Ah, got it. It will be inside the railsloader tempfile storage.

@tsun1215
Copy link
Author

tsun1215 commented May 5, 2015

Sorry about the response time. I don't think I'll have a lot of time until finals week is over (5/11).

@yasyf
Copy link

yasyf commented May 6, 2015

@tsun1215 what exactly do we want to accomplish with WatchStack? As in, once we have a list of changed constants, what do we want to do with them?

I've added #prepare_autoload and #process_autoload hooks (yasyf/rails@4dcc0e7) as well as an instance of WatchStack to the class so that we can do whatever we want around it. Right now I'm just printing out the new constants as a demo.

@tsun1215
Copy link
Author

tsun1215 commented May 6, 2015

Once we have a list of changed constants, we add them to the correct lists of constants so that they are removed when the rest of the autoloaded constants are removed.

@yasyf
Copy link

yasyf commented May 8, 2015

@tsun1215 cool so what I've got is a start to that. okay to merge it in to your stuff?

@tsun1215
Copy link
Author

Go for it, you have access

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants