Skip to content

prefer addon.moduleName as telemetry if one exists, otherwise fallback to packageName #38

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

Open
hmajoros wants to merge 4 commits into
base: master
Choose a base branch
from
Open

prefer addon.moduleName as telemetry if one exists, otherwise fallback to packageName #38

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
746e348
prefer addon.moduleName as telemetry if one exists, otherwise fallbac…
Feb 25, 2021
b361a14
Add Rob's suggestions in lib/utils/get-module-path-for.js
hmajoros Mar 26, 2021
fa4979f
bump eslint ecmaversion to allow optional catch binding
Mar 26, 2021
adefcb7
run prettier
Mar 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion lib/utils/get-module-path-for.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,32 @@ for (let packagePath of packagePaths) {
let packageDir = path.dirname(path.resolve(cwd, packagePath));

if (pkg.keywords && pkg.keywords.includes('ember-addon')) {
ADDON_PATHS[packageDir] = pkg.name;
// build addon instance
ADDON_PATHS[packageDir] = getAddonPackageName(packagePath);
Copy link

@tehhowch tehhowch Jan 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Should this be packageDir and not packagePath?

(If it is packagePath, this function dies attempting to require('package.json') on my team's addon, but if it is packageDir, then the native-class codemod is able to transform classes. Our setup is a multi-app monorepo that also contains a shared ember addon with reused components and services.)

} else if (isEmberCliProject(pkg)) {
APP_PATHS[packageDir] = pkg.name;
}
}

/**
* takes a package path and returns the runtime name of the addon.
*
* @param {String} packagePath the path on disk (from current working directory)
* @returns {String} The runtime name of the addon
*/
function getAddonPackageName(packagePath) {
const indexPath = path.resolve(path.dirname(packagePath), 'index.js');
Copy link
Member

@rwjblue rwjblue Mar 26, 2021

Choose a reason for hiding this comment

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

We shouldn't assume packagePath + index.js is the right addon entry point. We have to respect ember-addon.main in package.json.


if (fs.existsSync(indexPath)) {
const { name: packageName, moduleName } = require(indexPath);
// this is bad, fix later
return moduleName && typeof moduleName === 'function' ? moduleName() : packageName;
Copy link
Member

@rwjblue rwjblue Mar 26, 2021

Choose a reason for hiding this comment

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

We should probably do this in a try/catch block. In most cases that I've seen, the moduleName function is simple (e.g. it returns a string directly, without logic) but in some cases it is reasonable to access other context off of the addon instance (e.g. this.name). If an addon's moduleName implementation does this it will error and break telemetry gathering.

}

const { name: packageName } = require(packagePath);
return packageName;
}

function isEmberCliProject(pkg) {
return (
pkg &&
Expand Down