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

Improve global to import transform #131

Merged
merged 1 commit into from
May 25, 2016

Conversation

lemonmade
Copy link
Member

Fixes #99. Maybe fixes #95. This PR fixes files importing themselves by checking the expose directive (so, it must come after the default export transform, which it does), and adds a global cache that I believe will speed up the lookups between transforms.

cc/ @bouk @GoodForOneFare @Fandy

@bouk
Copy link
Contributor

bouk commented May 24, 2016

nice 👍

@@ -131,7 +152,7 @@ export default function globalReferenceToImport(
body,
j.importDeclaration([
j.importDefaultSpecifier(j.identifier(name)),
], j.literal(file))
], j.literal(relativePath(file)))
Copy link
Member

Choose a reason for hiding this comment

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

I think the snake case name will make it through to imports here.

Copy link
Member

Choose a reason for hiding this comment

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

My bad, the snake case is getting through via imports.set at line 104.

          const file = getDeclaringFile(member);
          if (file === null) { return null; }
          const name = findLastMember(node).name;
          imports.set(member, {file, name});  <------------ here.
          return name;

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I was reading the diff the wrong way round 😥 This is really fixing some imports where the snake case version was sneaking in. That's good!

Before/after this transform:

-import ApplePayCertificatePoller from 'admin/modules/apple_pay_certificate_poller';
+import ApplePayCertificatePoller from 'admin/modules/apple-pay-certificate-poller';

@GoodForOneFare
Copy link
Member

Code looks great. Just the snake case thing to fix before 🚢

I did a quick esify runthrough and everything was done in less time than the global import transform used to take on its own!! That's an outstanding result for a few shuffled lines 😁 Thank you!

@lemonmade lemonmade merged commit 40069fc into master May 25, 2016
@lemonmade lemonmade deleted the improve-global-to-import-transform branch May 25, 2016 15:59
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.

A file shouldn't try to import itself Make global-reference-to-import faster
3 participants