Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Load nothing from Tapioca by default #821

Merged
merged 2 commits into from
Feb 17, 2022
Merged

Conversation

paracycle
Copy link
Member

Motivation

The split between top level tapioca.rb requires and tapioca/internal.rb requires was creating an ordering problem. Also, the default require of Tapioca might be pulling into the requiring much more than we intended, and since we do messy things with Sorbet runtime, that could end up being distruptive.

Implementation

It is best to not load anything from the default require (we basically define the top-level Tapioca namespace and some constants under it, only) and to load everything for the CLI using tapioca/internal.rb. This allows us to order the requires so that all the trackers and other runtime patches get loaded before things that depend on them.

NOTE: This implies that people who write their own DSL compilers will have to explicitly do a require "tapioca/dsl/compiler".

Tests

No new tests.

@paracycle paracycle added the breaking-change Non-backward compatible change label Feb 16, 2022
@paracycle paracycle requested a review from a team February 16, 2022 23:31
Copy link
Collaborator

@Morriar Morriar left a comment

Choose a reason for hiding this comment

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

❤️

@paracycle paracycle force-pushed the uk-require-nothing-by-default branch from 0329dc3 to 9281492 Compare February 16, 2022 23:34
Sorbet runtime checks for method arguments change depending on if the
method arguments are all simple types and/or the return type is a simple
type or not.

Ever since we introduced the `TypeVariableModule` to make name binding
easier, type members/templates started being wrapped in simple types
when coerced, as well. However, this ends up breaking the assumptions
that Sorbet runtime is making, thus causing test failures for DSL
compilers with generic typed arguments in the constructor.

The solution is to hook the `T::Utils.coerce` method to coerce those
types to a custom `T::TypeVariable` subclass that records its name. By
doing this, we preserve the checks that Sorbet runtime is doing and we
preserve the name of the type variable.
@paracycle paracycle force-pushed the uk-require-nothing-by-default branch from 1f830ac to 9297b51 Compare February 17, 2022 00:35
@Morriar Morriar enabled auto-merge February 17, 2022 00:39
@Morriar Morriar merged commit 280feb4 into main Feb 17, 2022
@Morriar Morriar deleted the uk-require-nothing-by-default branch February 17, 2022 00:44
@shopify-shipit shopify-shipit bot temporarily deployed to production February 17, 2022 22:56 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to 0-7-stable March 29, 2022 19:23 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Non-backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants