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

[core] Add the download tracker package #33899

Merged
merged 4 commits into from Aug 15, 2022

Conversation

michaldudak
Copy link
Member

@michaldudak michaldudak commented Aug 12, 2022

Adds a no-op package to track downloads of @mui/material and @mui/joy, and, using this data, be able to figure out the usage of @mui/base alone.

As discussed in https://www.notion.so/mui-org/Define-a-one-year-goal-for-MUI-Core-07-2022-06-2023-d7a69c1f6a1c4f4aafc4c57061e6dbb0.

@michaldudak michaldudak added the core Infrastructure work going on behind the scenes label Aug 12, 2022
@michaldudak michaldudak requested a review from a team August 12, 2022 09:05
@mui-bot
Copy link

mui-bot commented Aug 12, 2022

No bundle size changes

Generated by 🚫 dangerJS against e05def2

@michaldudak michaldudak marked this pull request as ready for review August 12, 2022 09:54
package.json Outdated
@@ -7,7 +7,7 @@
"deduplicate": "node scripts/deduplicate.js",
"benchmark:browser": "yarn workspace benchmark browser",
"build:codesandbox": "lerna run --parallel --scope \"@mui/*\" build",
"release:version": "lerna version --no-changelog --no-push --no-git-tag-version",
"release:version": "lerna version --no-changelog --no-push --no-git-tag-version --force-publish=@mui/internal-usage-tracker",
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

LGTM, cc @mui/core if there are complains on the name. In my opinion it looks good.

@eps1lon
Copy link
Member

eps1lon commented Aug 12, 2022

This will remain no-op and just be used for install tracking i.e. anything that npm already does? You might want to add explicit docs for that in the package readme. Anything "tracking" will trigger privacy alarms otherwise

@michaldudak
Copy link
Member Author

michaldudak commented Aug 12, 2022

Good point, thanks.

@samuelsycamore, would you mind taking a look at the README file in this PR to whether it explains our intent clearly?

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

As discussed in https://www.notion.so/mui-org/Define-a-one-year-goal-for-MUI-Core-07-2022-06-2023-d7a69c1f6a1c4f4aafc4c57061e6dbb0.

I have made the page public

to figure out the usage of @mui/base alone.

Did we envision the implications of versioning? I mean, considering that the version of this package won't be bumped, does it mean that it will be more cached than our other developer-facing package, hence not downloaded, hence not increasing the npm download stats, hence not helping?

@@ -0,0 +1,5 @@
# @mui/internal-usage-tracker
Copy link
Member

Choose a reason for hiding this comment

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

Maybe? So it feels less frightening from a privacy perspective. No, it doesn't include telemetry.

Suggested change
# @mui/internal-usage-tracker
# @mui/internal-download-tracker

Same for the title of this PR for the changelog


Also from a namespace perspective, since @mui is shared between all products, maybe a prefix with core would help.

Copy link
Member

Choose a reason for hiding this comment

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

I like the download tracker since it sounds less scary at a glance

Copy link
Member

Choose a reason for hiding this comment

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

me too.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mui/core-internal-download-tracker, then?

@michaldudak
Copy link
Member Author

considering that the version of this package won't be bumped

It will be. The --force-publish flag makes sure the version of this package is updated regardless of changes detected in the package.

@michaldudak michaldudak changed the title [core] Add the usage tracker package [core] Add the download tracker package Aug 15, 2022
@michaldudak michaldudak merged commit afe281d into mui:master Aug 15, 2022
"private": false,
"author": "MUI Team",
"description": "Internal package to track number of downloads of our design system libraries",
"main": "./index.js",
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure about this one? It doesn't look CommonJS. I'm curious to see if it doesn't break some developers's bundle once released. I guess not since we don't import from it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing is ever imported from this package, so it shouldn't break.

daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
@michaldudak michaldudak deleted the usage-tracker-package branch January 23, 2023 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants