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

Add to UNUSED_EXTERNAL_IMPORT warning information about the origin of the problem #4054

Merged
merged 5 commits into from Apr 27, 2021
Merged

Add to UNUSED_EXTERNAL_IMPORT warning information about the origin of the problem #4054

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5abd833
feat: add sources to warn
cawa-93 Apr 21, 2021
6806159
fix: tests
cawa-93 Apr 21, 2021
33804d3
fix: semicolon
cawa-93 Apr 21, 2021
d542a45
refactoring
cawa-93 Apr 22, 2021
d5500f7
style: Add semicolons
cawa-93 Apr 22, 2021
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
20 changes: 18 additions & 2 deletions src/ExternalModule.ts
View file
Open in desktop
Expand Up @@ -110,11 +110,27 @@ export default class ExternalModule {
.map(name => `'${name}'`)
.join(', ')} and '${unused.slice(-1)}' are`;

const importersSet = new Set<string>()
for (const name of unused) {
const {importers, dynamicImporters} = this.declarations[name].module

if (Array.isArray(importers)) importers.forEach(v => importersSet.add(v))
if (Array.isArray(dynamicImporters)) dynamicImporters.forEach(v => importersSet.add(v))
Copy link
Member

Choose a reason for hiding this comment

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

By the types, these checks should not be needed. This is also reflected in the coverage right now. Or do you see a scenario where there might be an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

During my previous experiments there was a situation when dynamicImporters was undefined. I assume that the types are not quite correct. Therefore, for reinsurance, I added a check.

}

const importersArray = Array.from(importersSet)

let importerList = ''
if (importersArray.length > 0) {
cawa-93 marked this conversation as resolved.
Show resolved Hide resolved
importerList = ' in' + importersArray.map(s => `\n\t${s};`)
}

this.options.onwarn({
code: 'UNUSED_EXTERNAL_IMPORT',
message: `${names} imported from external module '${this.id}' but never used`,
message: `${names} imported from external module '${this.id}' but never used${importerList}`,
names: unused,
source: this.id
source: this.id,
sources: importersArray
Copy link
Contributor Author

@cawa-93 cawa-93 Apr 21, 2021 •
edited

Choose a reason for hiding this comment

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

I'm not sure how appropriate to use sources key. I did not find any description of the properties of the RollupWarning interface.

rollup/src/rollup/types.d.ts

Lines 9 to 22 in a4a3b8a

export interface RollupWarning extends RollupLogProps {
chunkName?: string;
cycle?: string[];
exporter?: string;
exportName?: string;
guess?: string;
importer?: string;
missing?: string;
modules?: string[];
names?: string[];
reexporter?: string;
source?: string;
sources?: string[];
}

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the other usages, sources should be reasonable.

});
}
}
7 changes: 6 additions & 1 deletion test/function/samples/unused-import/_config.js
View file
Open in desktop
@@ -1,3 +1,5 @@
const path = require('path')

module.exports = {
description: 'warns on unused imports ([#595])',
options: {
Expand All @@ -13,7 +15,10 @@ module.exports = {
code: 'UNUSED_EXTERNAL_IMPORT',
source: 'external',
names: ['notused', 'neverused'],
message: `'notused' and 'neverused' are imported from external module 'external' but never used`
message: `'notused' and 'neverused' are imported from external module 'external' but never used in\n\t${path.resolve(__dirname, './main.js')};`,
sources: [
path.resolve(__dirname, './main.js')
]
}
]
};