[docs-infra] Move ads to the @mui/docs package

[alexfauquette]

Followup of #42842

commits

1. Move files to the package
2. Use context for configuration of ads
3. re-export the Ad component to avoid breaking toolpad docs

https://github.com/mui/mui-toolpad/blob/4298328c314621b0533974590284eb70f3dfffd9/docs/src/modules/components/SchemaReference.tsx#L4

[mui-bot]

Netlify deploy preview

https://deploy-preview-42944--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 72c600f

[alexfauquette]

I don't know why moving those files to the package results in the following error

src/Ad/ad.styles.ts(95,14): error TS2742: The inferred type of 'adStylesObject' cannot be named without a reference to 'packages/mui-styled-engine/node_modules/csstype'. This is likely not portable. A type annotation is necessary.
buildTypes.mjs

Not proud of my fix: 0aa6cf4
I probably missed something

[Janpot]

I probably missed something

Have you tried adding a reference to @mui/styled-engine here?

[alexfauquette]

Have you tried adding a reference to @mui/styled-engine here?

It did not worked (or I don't know how to do it correctly)

After some code commenting, it appears that the issue does not come from the styled(...)(problematicFunction) which infers the typing of the problematicFunction.

It's just the theme.typographe.body1 and other typography that requires the font typing which are in csstypes So I just added the library to the tsconfig

[oliviertassinari]

I'm not following why this should be configurable. Can we hardcode it as a constant?

Suggested change

* @default 0.1

[Janpot]

We may want it to be different for different products? e.g. Toolpad docs has a lot less traffic.

[oliviertassinari]

I'm not really seeing the use case.

I have introduced this sampling because we didn't need to get all the events, we need enough to get statistical significance in the data. I started to collect this data to know the actual distribution of the ad display types. It's meant to make sure that reality matches the configuration intent.

I had surprises once I looked at the data in GA, there were bugs, and the actual ad displayed types were different from what I tried to implement. It took me some time, but I eventually found the bugs. For this, it doesn't matter if the ad is shown in toolpad, base ui, as long as we don't allow toolpad, base ui, etc. to configure the ad display types.

[Janpot]

👍 All fine for me, I'm just stating a potential use-case. Personally I don't care for it neither.

[alexfauquette]

I don't see lot of usecases, so I'm ok to revert this. But IMO this migration to a dedicated package is an opportunity to find (and remove) all the hardcoded piece of code, and find clean way to make them configurable

[oliviertassinari]

all the hardcoded piece of code, and find clean way to make them configurable

@alexfauquette I agree with the strategy. In the tactics we use to do this, I think we should focus on looking at MUI X, Toolpad, Base UI docs in this order:

1. see what we had to duplicate because we couldn't configure it. Very clear signal.
2. see what we wish we were able to duplicate but couldn't, and this led to problematic UX. Clear signal.
3. see what we might need to configure in the future. Weak signal. Only worth it if there are no doubts?

This ad constant seems to be case 3.

[alexfauquette]

Only worth it if there are no doubts?

I've seen it as a nice room for experimentation. If this implementation is causing trouble, we can easily replace it by a dumb constant. If the customization was needed for toolpad, base, or X it would be a bigger issue to fix.

[LukasTy]

@mui/code-infra WDYT, would it make sense to specify such types in dependencies to avoid the need for explicit definition on the consuming side? 🤔
https://github.com/mui/mui-x/pull/13999/files#diff-adfa337ce44dc2902621da20152a048dac41878cf3716dfc4cc56d03aa212a56

[Janpot]

I think it could make sense once @mui/docs adopts the _document.js implementation (or wherever the GA script is initialized). I'd avoid it to have dependencies declare global variables on dependant projects withoit guaranteeing those will exist.