Skip to content

Fix minified variable name collision in a type generic factory function. #1237

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

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

nevkontakte
Copy link
Member

@nevkontakte nevkontakte commented Oct 27, 2023

Prior to this change we always canonicalized method's type parameter names to their receiver counterparts. This was mainly necessary to avoid errors when we needed to reference method's type parameters inside a type generic factory function, e.g. when initializing method's reflection metadata.

Such implementation, however, was causing a bug when minification is enabled:

  1. Methods are compiled first, variable names are allocated to their type parameters and the association is stored in the package context.
  2. The allocated variable name is marked as in use within the mathod's functionContext, up to the enclosing generic factory function context, but not at the package level.
  3. Type definitions are compiled second. Because the type's typeparams are already associated with variable names, these variable names are used. The process of allocating a new variable is omitted and the previously allocated name is not marked as allocated within the type's generic factory function.
  4. A new variable inside the type's generic factory function is allocated, and its name may collide with the type param's variable leading to errors.

With this change, canonicalization is only done if we are outside of the method's functionContext. That way no variable name is associated with type's typeparam objects while compiling the method, and the JS variable is correctly allocated and marked as in use.

Updates #1013

@nevkontakte nevkontakte requested a review from flimzy October 27, 2023 20:49
Prior to this change we always canonicalized method's type parameter
names to their receiver counterparts. This was mainly necessary to avoid
errors when we needed to reference method's type parameters inside a
type generic factory function, e.g. when initializing method's
reflection metadata.

Such implementation, however, was causing a bug when minification is
enabled:

 1. Methods are compiled first, variable names are allocated to their
    type parameters and the association is stored in the package
    context.
 2. The allocated variable name is marked as in use within the mathod's
    functionContext, up to the enclosing generic factory function
    context, but not at the package level.
 3. Type definitions are compiled second. Because the type's typeparams
    are already associated with variable names, these variable names are
    used. The process of allocating a new variable is omitted and the
    previously allocated name is not marked as allocated within the
    type's generic factory function.
 4. A new variable inside the type's generic factory function is
    allocated, and its name may collide with the type param's variable
    leading to errors.

With this change, canonicalization is only done if we are outside of
the method's functionContext. That way no variable name is associated
with type's typeparam objects while compiling the method, and the JS
variable is correctly allocated and marked as in use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants