Skip to content

Conversation

@KaanOzkan
Copy link
Contributor

Extensions may override DSL compilers and should be loaded before so that abstract methods such as gather_constants and decorate of the original compilers take precedence

These require calls were loading the compilers before the extensions which isn't the case for using tapioca from the CLI.

Without the requires the code should still work since addon.rb will dispatch load_compilers_and_extensions request before route_dsl or fixtures_dsl.

Extensions may override DSL compilers and should be loaded before so
that abstract methods such as `gather_constants` and `decorate` of the
original compilers take precedence

These `require` calls were loading the compilers before the extensions
which isn't the case for using tapioca from the CLI. Without the
requires the code should still work since `addon.rb` will dispatch
`load_compilers_and_extensions` request before `route_dsl` or
`fixtures_dsl`.
@KaanOzkan KaanOzkan requested a review from a team as a code owner June 12, 2025 20:44
@KaanOzkan KaanOzkan changed the title Do not load DSL compilers before extensions Do not load DSL compilers before extensions in add-on mode Jun 12, 2025
@paracycle
Copy link
Member

The changes look good to me, but I don't understand this statement:

Extensions may override DSL compilers and should be loaded before so that abstract methods such as gather_constants and decorate of the original compilers take precedence

Which extensions override DSL compilers? They certainly need to be loaded before compilers since they are meant to collect information that compilers will ultimately use, but they really should not be doing any overriding of anything.

# frozen_string_literal: true

require "tapioca/internal"
require "tapioca/dsl/compilers/url_helpers"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these requires were specifically added to fix something #2165. Are we not reintroducing the same issue?

Copy link
Contributor Author

@KaanOzkan KaanOzkan Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, we still have the main code that runs gather_constants for both compilers. I think require was added since we're used to doing it before referencing a constant but it's not necessary (unless there's a change in how activate works). Goal is to have the order:

  1. Load this file (server_addon.rb)
  2. Run activate in addon_rb which runs the load_compilers_and_extensions request
  3. load_compilers_and_extensions loads all tapioca compilers
  4. Routes or Fixture edits reference the compilers that are already loaded.

@KaanOzkan KaanOzkan merged commit d9b45f9 into main Jun 12, 2025
18 of 19 checks passed
@KaanOzkan KaanOzkan deleted the ko/fix-tapioca-dsl-arfixtures branch June 12, 2025 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants