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

[tools] Add new page for Base UI npm downloads KPIs #102

Merged
merged 4 commits into from
May 25, 2023

Conversation

mnajdova
Copy link
Member

Added new page for Base UI's npm KPIs.

image

@mnajdova mnajdova requested a review from oliviertassinari May 17, 2023 10:49
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.

Sweet 👍

Comment on lines +25 to +45
export const prepareData = (inData) => {
const date = new Date(2022, 6, 1, 0, 0, 0, 0);
const today = new Date();
const packages = getPackages(inData);

const monthsData = {};

while(date < today) {
monthsData[getDateString(date)] = {};
packages.forEach(packageName => {
monthsData[getDateString(date)][packageName] = 0;
})
date.setMonth(date.getMonth() + 1);
}

packages.forEach(packageName => {
Object.keys(inData[packageName]).map(date => {
const monthKey = getMonthKey(date);
monthsData[monthKey][packageName] += inData[packageName][date];
})
});
Copy link
Member

@oliviertassinari oliviertassinari May 17, 2023

Choose a reason for hiding this comment

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

Oh nice, it looks almost like the logic in https://docs.google.com/spreadsheets/d/1FHxZ456e1t_MNJZg9wrq7pLRFzknZ6jMBhFuW1j1Q8o/edit#gid=0 used to fetch the data (importdownload()).

function importdownload(start = '2023-01-01', end = '2023-06-01', packagesRange = [['react-dom', '@mui/base']]) {
  const packages = packagesRange[0].filter((cell) => cell !== '');
  console.log('URL', `https://npm-stat.com/api/download-counts?${addQuery({
    package: packages
  })}&from=${start}&until=${end}`);

  var jsondata = UrlFetchApp.fetch(`https://npm-stat.com/api/download-counts?${addQuery({
    package: packages
  })}&from=${start}&until=${end}`);

  let npmStateResponse = JSON.parse(jsondata.getContentText());

  const months = {};

  Object.keys(npmStateResponse).forEach((package) => {
    const downloads = npmStateResponse[package];

    Object.keys(downloads).forEach((date) => {
      const month = date.substring(0, 7);

      months[month] = months[month] || {};
      months[month][package] = months[month][package] || 0;
      months[month][package] += downloads[date];
    });
  });

  const output = [['month', ...packages]];

  Object.keys(months).sort().forEach((month) => {
    output.push([month, ...packages.map((package) => {
      if (package === 'react-dom') {
        return months[month][package];
      }
      return months[month][package] / months[month]['react-dom'];
    })]);
  });

  console.log('output', output);
  return output;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, looks like the same thing 👍 I was adding to add similar page for all products

const entry = {
date,
...monthsData[date],
'@mui/base': monthsData[date]['@mui/base'] + monthsData[date]['@mui/core'] - monthsData[date]['@mui/material'],
Copy link
Member

Choose a reason for hiding this comment

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

A side note, in https://docs.google.com/spreadsheets/d/1FHxZ456e1t_MNJZg9wrq7pLRFzknZ6jMBhFuW1j1Q8o/edit#gid=590909841, I also subtract with @mui/joy, underestimating the actual downloads of Base UI. Soon or later, we can move to @mui/core-downloads-tracker

Screenshot 2023-05-17 at 19 05 21

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 will leave it as is for now

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, agree, makes more sense.

{field: 'ratio', headerName: "Base UI marketshare", width: 150}
]);

function DownloadsTable({ data: dataProp }: DownloadsTableProps) {
Copy link
Member

Choose a reason for hiding this comment

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

This feels like a component that would be better in the Toolpad editor, getting us a bit closer to the Google Sheet experience.

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 don't understand, what do you mean in the Toolpad editor?

Copy link
Member

@Janpot Janpot May 18, 2023

Choose a reason for hiding this comment

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

I'm interested in learning about why you went for a code component to implement this table rather than use the table that is built in Toolpad. Looking at the code it seems like you use the code component mainly to preprocess the incoming data. At a glance, the problems to solve for us are:

  • Computed columns
    Adding columns mapped from the raw rows, but statically known. e.g. "date" and "ratio". I believe we could solve this either by providing a low-code way of remapping data, or by supporting computed columns
  • Dynamically created columns
    Creating new columns dynamically based on incoming data. e.g. adding a column for each package. It's not clear to me yet how we can support this comprehensively, would need to benchmark other tools for ideas
  • reuse mapped data
    This is a problem I don't see directly here, but we need to think about how we can support e.g. showing a graph of the filtered data of the datagrid. Maybe we can think about providing a myGrid.filteredRows or something that other components can bind to. Not fully convinced about this yet as it tightly couples components to each other. (What if I want to delete/replace the datagrid, does the chart become useless? Do I have to redo all data mangling?)
    Perhaps we should offer an option to circumvent the static column definition and always infer from the incoming data?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see the question, initially I was structuring the data in the component itself, but then moved this logic in a serverless function, I will try to do this change and report back if I miss a feature.

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've updated the app to use the DataGrid component directly. The only feature I was missing (or didn't know how to do) is rename the title of the columns

@@ -0,0 +1,58 @@
import * as React from "react";
import { createComponent } from "@mui/toolpad/browser";
import { LineChart, Line, CartesianGrid, XAxis, Tooltip, Legend, YAxis } from 'recharts';
Copy link
Member

@oliviertassinari oliviertassinari May 17, 2023

Choose a reason for hiding this comment

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

Soon https://master--material-ui-x.netlify.app/x/react-charts/lines/#data-format 😁

Actually, the API would be different, it wouldn't work like recharts

<LineChart width={600} height={300} data={data}>

@oliviertassinari oliviertassinari changed the title [tools-public] Add new page for Base UI npm downloads KPIs [tools] Add new page for Base UI npm downloads KPIs May 18, 2023
@oliviertassinari oliviertassinari added the new feature New feature or request label May 18, 2023
Comment on lines 10 to 16
const getColumns = (packages) => ([
{ field: 'date', headerName: 'Month', width: 150 },
...packages.map(packageName => ({
field: packageName, headerName: packageName, width: packageName === '@radix-ui/react-primitive' || packageName === '@headlessui/react' ? 170 : 120
})),
{field: 'ratio', headerName: "Base UI marketshare", width: 150}
]);
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
const getColumns = (packages) => ([
{ field: 'date', headerName: 'Month', width: 150 },
...packages.map(packageName => ({
field: packageName, headerName: packageName, width: packageName === '@radix-ui/react-primitive' || packageName === '@headlessui/react' ? 170 : 120
})),
{field: 'ratio', headerName: "Base UI marketshare", width: 150}
]);
const getColumns = (packages) => ([
{ field: 'date', headerName: 'Month', width: 150 },
...packages.map(packageName => ({
field: packageName, headerName: packageName, width: 150
})),
{field: 'ratio', headerName: "Base UI marketshare", width: 150}
]);

cc @apedroferreira this is how you can reproduce the issue with the table overflowing.

Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

Nice!

On Toolpad side, I think feature-wise there could be focus on

  • The stats component and the chart component seem to be the two main reasons why internally we keep creating custom components
  • select/autocomplete options, datagrid rows, list data,... I always see big expressions to filter/map query data into lists of objects.

@mnajdova
Copy link
Member Author

mnajdova commented May 23, 2023

@Janpot I didn't see that you approved the PR and in the mean time I added one more page. Could you do a quick check please! Thanks!

Here is a screenshot:
image

I've reused the existing NPM chart component. It took minutes to add this new page, I am pretty happy with it

@mnajdova
Copy link
Member Author

I am merging, next I will be adding some dashboards using the Google analytics API.

@mnajdova mnajdova merged commit 6acc4c0 into mui:master May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants