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

fix: conflict between imported and local binding #356

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

hyf0
Copy link
Member

@hyf0 hyf0 commented Nov 22, 2023

Description

Fixes #355

Test Plan


@hyf0
Copy link
Member Author

hyf0 commented Nov 22, 2023

@hyf0 hyf0 force-pushed the 11-22-fix_conflict_between_imported_and_local_binding branch from 3b9c675 to eff27a1 Compare November 22, 2023 06:57
self.imports_from_other_chunks.iter().flat_map(|(_, items)| items.iter()).for_each(|item| {
renamer.add_top_level_symbol(item.import_ref);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain it why does not affect? If add_top_level_symbol order will cause some error, maybe move it up also has some side effects.

Copy link
Member Author

@hyf0 hyf0 Nov 22, 2023

Choose a reason for hiding this comment

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

maybe move it up also has some side effects.

Yeah. You are totally right. Moving up fix the problem in #355.

To avoid conflict with imported names, we should first reserve the names of imports, but the old code doesn't do it first instead do it secondly, so it has no effect.

@hyf0
Copy link
Member Author

hyf0 commented Nov 22, 2023

Merge activity

  • Nov 22, 6:57 AM: @hyf0 started a stack merge that includes this pull request via Graphite.
  • Nov 22, 6:57 AM: @hyf0 merged this pull request with Graphite.

@hyf0 hyf0 merged commit 7387e35 into main Nov 22, 2023
6 checks passed
@hyf0 hyf0 deleted the 11-22-fix_conflict_between_imported_and_local_binding branch November 22, 2023 11:57
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.

None yet

2 participants