tapioca icon indicating copy to clipboard operation
tapioca copied to clipboard

Make `require.rb` less necessary

Open paracycle opened this issue 4 years ago • 8 comments

One of the main stumbling blocks for people getting started with Tapioca is around the require.rb file. There are a few things we can do to improve the brittleness around it, some of which are:

  • [x] Require all gems in the Gemfile regardless of require: false.

    This is a bit hacky, but most of our require.rb files are dominated by the requires for gems that have been marked as require: false in the Gemfile. If we can make Bundler load those gems as well, then those entries in the require.rb file can just go away.

  • [x] Add built-in knowledge of some of the non-default requires of popular gems.

    Like how Sorbet srb rbi tooling does, we can start maintaining a list of gems with their non-standard (optional) requires that we can use instead of asking everyone to keep redeclaring those again and again. Ideally, this would not be bundled into the Tapioca gem, but can be distributed out-of-band and fetched using a Tapioca command (maybe tapioca require can do that?)

  • [ ] Try to brute force loading as many files from the gem lib path as possible.

    We could try to load all the files from a gem's lib directory, but we have to be careful about exit/abort calls, at_exit hooks, etc etc and we also need to probably make a few passes through the requires to make sure that the include order related issues can be resolved as much as possible.

paracycle avatar Aug 12 '21 13:08 paracycle

See also https://github.com/sorbet/sorbet/blob/master/gems/sorbet/lib/gem_loader.rb

Morriar avatar Aug 23 '21 15:08 Morriar

Because we sort the requires, it may sometimes result in errors if they need a specific order. Currently, it's a confusing experience for developers, since they often just see a message about missing constants and don't realize what's happening. Ideas for improving the experience:

  • Better documentation around tapioca require
  • Rescue LoadError and provide a better tailored error message, mentioning that their require.rb file has a mistake and maybe even pointing them to documentation about how to fix it

vinistock avatar Aug 23 '21 16:08 vinistock

Can you please explain which problems people have with the require.rb file?

rafaelfranca avatar Sep 07 '21 20:09 rafaelfranca

Can you please explain which problems people have with the require.rb file?

I see two main sources of confusion:

  1. When a constant or method is missing it generally requires an understanding of the gems internals to know what to require. It puts burden on the user that has to go read the gem's source to find where the constant is declared and which file should be added to Tapioca's require.rb file.

  2. When the require.rb file is auto-populated with tapioca requires and an entry is incorrect, users have a hard time figuring what should be removed from the file and why. We've seen occurrences where people were regenerating the require.rb file between each gem bump, re-adding the same erroneous requires over and over for example.

Vini's comment explains one more limitation, the ordering of the requires that might also be problematic.

All of this shows that the current approach, while working, might not be the easiest one from a user point of view.

Morriar avatar Sep 13 '21 15:09 Morriar

@Morriar you and Ufuk have a branch somewhere with a prototype for this, right? If so, can you link it here?

dirceu avatar Aug 08 '23 15:08 dirceu

@Morriar you and Ufuk have a branch somewhere with a prototype for this, right? If so, can you link it here?

It's very WIP but it's here: https://github.com/Shopify/tapioca/commit/e6e1dba26a3a63c2c33c71732f6073da00eb05d6.

Morriar avatar Aug 08 '23 16:08 Morriar

We've prototyped the existing autoload solution linked above and gathered some timings. It was clear that using sorbet's symbol table to identify constants to load was too slow. An early prototype using RubyIndexer showed much faster results.

Prototype also revealed that behaviour of the autoload has slightly changed because we're running into stack overflow during loading itself. This wasn't the case when the solution was tested the first time.

Next steps are:

  1. Modifying SymbolLoader to identify constants using RubyIndexer instead of Sorbet's symbol table to bring the speedups to existing tapioca implementation
  2. Reuse the new SymbolLoader logic to identify constants for autoloading and compare the performance between identifying all constants (for autoloading) vs identifying constants we're generating RBIs for
  3. Investigate the stack overflow error during autoloading (constantize)

After these steps we could compare autoloading solution and a static list of requires. I'm skeptical autoloading will remove the need for require.rb completely and would like to see it in action.

KaanOzkan avatar Aug 23 '23 14:08 KaanOzkan

  1. Modifying SymbolLoader to identify constants using RubyIndexer instead of Sorbet's symbol table to bring the speedups to existing tapioca implementation

We tested bootstrapping symbols using RubyIndexer and this new approach didn't result in any performance benefits.

We still have to investigate and resolve the stack overflow error so that we can test autoloading solution in practice.

KaanOzkan avatar Sep 14 '23 21:09 KaanOzkan