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: remove extra imports for mixin dependencies #1641

Merged
merged 2 commits into from
Aug 25, 2018
Merged

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Aug 24, 2018

With the new TypeScript version, it is no longer necessary to manually import types referenced by mixin classes.

This commit removes those extra imports from the CLI template and scaffolded "application.ts" files; and also sorts import using VS Code's feature "Organize imports".

I have discovered this opportunity while working on service-registration mixin (#1439).

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@bajtos bajtos added this to the August Milestone milestone Aug 24, 2018
@bajtos bajtos self-assigned this Aug 24, 2018
@bajtos bajtos requested a review from raymondfeng as a code owner August 24, 2018 13:32
Copy link
Contributor

@virkt25 virkt25 left a comment

Choose a reason for hiding this comment

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

YAY for this finally being fixed with TypeScript. 😍

@bajtos
Copy link
Member Author

bajtos commented Aug 24, 2018

Hmm, I need to fix few more files:

  • packages/repository/README.md
  • legacy-juggler-bridge in repository package
  • "src/keys" in rest package
  • packages/repository/test/unit/query/query-builder.unit.ts

bajtos added 2 commits August 24, 2018 16:01
With the new TypeScript version, it is no longer necessary to manually
import types referenced by mixin classes.

This commit removes those extra imports from the CLI template and
scaffolded "application.ts" files; and also sorts import using VS Code's
feature "Organize imports".

While removing unused imports, I cleaned up few more places where
we were importing extra types.
"include": [
"benchmark",
"examples",
"packages"
Copy link
Member Author

Choose a reason for hiding this comment

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

I have sorted the entries alphabetically.

@bajtos
Copy link
Member Author

bajtos commented Aug 24, 2018

The patch is ready for the final review and landing. @strongloop/sq-lb-apex @raymondfeng PTAL.

Copy link
Contributor

@raymondfeng raymondfeng left a comment

Choose a reason for hiding this comment

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

Nice!

@raymondfeng raymondfeng merged commit f5fc74f into master Aug 25, 2018
@raymondfeng raymondfeng deleted the simplify-imports branch August 25, 2018 00:18
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.

4 participants