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.

Sign up for GitHub

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

Jump to bottom

Refactor isImportTypeOnly helper function #10047

Merged
merged 1 commit into from May 31, 2019
Merged

Refactor isImportTypeOnly helper function #10047

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 10 additions & 13 deletions packages/babel-plugin-transform-typescript/src/index.js
View file
Open in desktop
Expand Up @@ -17,12 +17,7 @@ function isInType(path) {
}
}

interface State {
programPath: any;
}

const PARSED_PARAMS = new WeakSet();
const PRAGMA_KEY = "@babel/plugin-transform-typescript/jsxPragma";

export default declare((api, { jsxPragma = "React" }) => {
api.assertVersion(7);
Expand All @@ -39,16 +34,15 @@ export default declare((api, { jsxPragma = "React" }) => {
Identifier: visitPattern,
RestElement: visitPattern,

Program(path, state: State) {
state.programPath = path;
Copy link
Member Author

Choose a reason for hiding this comment

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

this was some leftover, not used anymore


Program(path, state) {
const { file } = state;
let fileJsxPragma = null;

if (file.ast.comments) {
for (const comment of (file.ast.comments: Array<Object>)) {
const jsxMatches = JSX_ANNOTATION_REGEX.exec(comment.value);
if (jsxMatches) {
file.set(PRAGMA_KEY, jsxMatches[1]);
fileJsxPragma = jsxMatches[1];
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of storing this in a shared mutable state under some artificial key I think it's better to just pass it as explicit argument to isImportTypeOnly, as it makes the data flow clearer - especially that this info doesn't have to be passed through layers of functions, it's needed just one 1 level lower.

}
}
}
Expand Down Expand Up @@ -76,7 +70,11 @@ export default declare((api, { jsxPragma = "React" }) => {
// import anyway.
if (
binding &&
isImportTypeOnly(file, binding, state.programPath)
isImportTypeOnly({
binding,
programPath: path,
jsxPragma: fileJsxPragma || jsxPragma,
})
) {
importsToRemove.push(binding.path);
} else {
Expand Down Expand Up @@ -327,15 +325,14 @@ export default declare((api, { jsxPragma = "React" }) => {
// 'access' and 'readonly' are only for parameter properties, so constructor visitor will handle them.
}

function isImportTypeOnly(file, binding, programPath) {
function isImportTypeOnly({ binding, programPath, jsxPragma }) {
for (const path of binding.referencePaths) {
if (!isInType(path)) {
return false;
}
}

const fileJsxPragma = file.get(PRAGMA_KEY) || jsxPragma;
if (binding.identifier.name !== fileJsxPragma) {
if (binding.identifier.name !== jsxPragma) {
return true;
}

Expand Down