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

Addon-docs: Resolve mdx-react-shim & @storybook/global correctly #23941

Merged
merged 8 commits into from
Aug 25, 2023
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,5 @@ code/playwright-results/
code/playwright-report/
code/playwright/.cache/
code/bench-results/

/packs
6 changes: 5 additions & 1 deletion code/addons/docs/src/preset.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from 'fs-extra';
import { dirname, join } from 'path';
import remarkSlug from 'remark-slug';
import remarkExternalLinks from 'remark-external-links';
import { dedent } from 'ts-dedent';
Expand Down Expand Up @@ -50,7 +51,10 @@ async function webpack(
skipCsf: true,
...mdxPluginOptions,
mdxCompileOptions: {
providerImportSource: '@storybook/addon-docs/mdx-react-shim',
providerImportSource: join(
dirname(require.resolve('@storybook/addon-docs/package.json')),
'/dist/shims/mdx-react-shim'
),
...mdxPluginOptions.mdxCompileOptions,
remarkPlugins: [remarkSlug, remarkExternalLinks].concat(
mdxPluginOptions?.mdxCompileOptions?.remarkPlugins ?? []
Expand Down
9 changes: 7 additions & 2 deletions code/addons/essentials/src/docs/preset.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
/* eslint-disable import/export */
import { dirname, join } from 'path';

// eslint-disable-next-line import/export
export * from '@storybook/addon-docs/dist/preset';

export const mdxLoaderOptions = async (config: any) => {
// eslint-disable-next-line no-param-reassign
config.mdxCompileOptions.providerImportSource = '@storybook/addon-essentials/docs/mdx-react-shim';
config.mdxCompileOptions.providerImportSource = join(
dirname(require.resolve('@storybook/addon-docs/package.json')),
'/dist/shims/mdx-react-shim'
);
Copy link
Member

@IanVS IanVS Aug 25, 2023

Choose a reason for hiding this comment

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

Why do we need to use /dist/shims here, when the exports of addon-docs includes mdx-react-shim ? I thought the way we normally handle bundlers that don't support exports (webpack and vite both do, though) was by putting a file at the root which re-exported from dist. That feels like a cleaner solution to me, rather than sprinkling dist throughout our source.

Copy link
Member Author

Choose a reason for hiding this comment

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

@IanVS right, I'd definitely not want users to import directly from dist, because what's in dist is internal for us.
But that's also the reaosn why I think here it's OK to use:
This is all contained in the monorepo, so I'm less afraid this will break.

Sure I could direct to a root file, and then the root file directs to dist.. but I'd rather have less hoops and less files that need to be there for obscure reasons.

If this works, and therefore 1 less reason for having an root files that's 👍 for me.

I'm hoping that long term we won't have any root files anymore.. but that's likely going to be around the same time when:

we want to support true node.js ESM, we'll need to start using createRequire, since our .mjs files that are generated right now will throw errors if require is not available, but that can be a matter for another day.

return config;
};
6 changes: 5 additions & 1 deletion code/builders/builder-vite/src/plugins/mdx-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Plugin } from 'vite';
import remarkSlug from 'remark-slug';
import remarkExternalLinks from 'remark-external-links';
import { createFilter } from 'vite';
import { dirname, join } from 'path';

const isStorybookMdx = (id: string) => id.endsWith('stories.mdx') || id.endsWith('story.mdx');

Expand Down Expand Up @@ -33,7 +34,10 @@ export async function mdxPlugin(options: Options): Promise<Plugin> {
const mdxLoaderOptions = await options.presets.apply('mdxLoaderOptions', {
...mdxPluginOptions,
mdxCompileOptions: {
providerImportSource: '@storybook/addon-docs/mdx-react-shim',
providerImportSource: join(
dirname(require.resolve('@storybook/addon-docs/package.json')),
'/dist/shims/mdx-react-shim'
),
...mdxPluginOptions?.mdxCompileOptions,
remarkPlugins: [remarkSlug, remarkExternalLinks].concat(
mdxPluginOptions?.mdxCompileOptions?.remarkPlugins ?? []
Expand Down
2 changes: 2 additions & 0 deletions code/lib/preview/src/globals/runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as CHANNELS from '@storybook/channels';
import * as CLIENT_LOGGER from '@storybook/client-logger';
import * as CORE_EVENTS from '@storybook/core-events';
import * as PREVIEW_API from '@storybook/preview-api';
import * as GLOBAL from '@storybook/global';

// DEPRECATED, remove in 8.0
import * as ADDONS from '@storybook/preview-api/dist/addons';
Expand All @@ -22,6 +23,7 @@ export const values: Required<Record<keyof typeof globals, any>> = {
'@storybook/client-logger': CLIENT_LOGGER,
'@storybook/core-events': CORE_EVENTS,
'@storybook/preview-api': PREVIEW_API,
'@storybook/global': GLOBAL,

// DEPRECATED, remove in 8.0
'@storybook/addons': ADDONS,
Expand Down
1 change: 1 addition & 0 deletions code/lib/preview/src/globals/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Here we map the name of a module to their NAME in the global scope.
export const globals = {
'@storybook/addons': '__STORYBOOK_MODULE_ADDONS__',
'@storybook/global': '__STORYBOOK_MODULE_GLOBAL__',
'@storybook/channel-postmessage': '__STORYBOOK_MODULE_CHANNEL_POSTMESSAGE__', // @deprecated: remove in 8.0
'@storybook/channel-websocket': '__STORYBOOK_MODULE_CHANNEL_WEBSOCKET__', // @deprecated: remove in 8.0
'@storybook/channels': '__STORYBOOK_MODULE_CHANNELS__',
Expand Down
23 changes: 21 additions & 2 deletions scripts/run-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import program from 'commander';
import { runServer, parseConfigFile } from 'verdaccio';
import pLimit from 'p-limit';
import type { Server } from 'http';
import { mkdir } from 'fs/promises';
import { PACKS_DIRECTORY } from './utils/constants';
// @ts-expect-error (Converted from ts-ignore)
import { maxConcurrentTasks } from './utils/concurrency';
import { listOfPackages } from './utils/list-packages';
Expand Down Expand Up @@ -53,12 +55,27 @@ const currentVersion = async () => {
return version;
};

const publish = (packages: { name: string; location: string }[], url: string) => {
const publish = async (packages: { name: string; location: string }[], url: string) => {
logger.log(`Publishing packages with a concurrency of ${maxConcurrentTasks}`);

const limit = pLimit(maxConcurrentTasks);
let i = 0;

/**
* We need to "pack" our packages before publishing to npm because our package.json files contain yarn specific version "ranges".
* such as "workspace:*"
*
* We can't publish to npm if the package.json contains these ranges. So with `yarn pack` we create a tarball that we can publish.
*
* However this bug exists in NPM: https://github.com/npm/cli/issues/4533!
* Which causes the NPM CLI to disregard the tarball CLI argument and instead re-create a tarball.
* But NPM doesn't replace the yarn version ranges.
*
* So we create the tarball ourselves and move it to another location on the FS.
* Then we change-directory to that directory and publish the tarball from there.
*/
await mkdir(PACKS_DIRECTORY, { recursive: true }).catch(() => {});

return Promise.all(
packages.map(({ name, location }) =>
limit(
Expand All @@ -70,7 +87,9 @@ const publish = (packages: { name: string; location: string }[], url: string) =>
'.'
)})`
);
const command = `cd ${location} && yarn pack && npm publish ./package.tgz --registry ${url} --force --access restricted --ignore-scripts && rm ./package.tgz`;

const tarballFilename = `${name.replace('@', '').replace('/', '-')}.tgz`;
const command = `cd ${location} && yarn pack --out=${PACKS_DIRECTORY}/${tarballFilename} && cd ${PACKS_DIRECTORY} && npm publish ./${tarballFilename} --registry ${url} --force --access restricted --ignore-scripts`;
exec(command, (e) => {
if (e) {
rej(e);
Expand Down
2 changes: 2 additions & 0 deletions scripts/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { join } from 'path';
export const AFTER_DIR_NAME = 'after-storybook';
export const BEFORE_DIR_NAME = 'before-storybook';

export const ROOT_DIRECTORY = join(__dirname, '..', '..');
export const CODE_DIRECTORY = join(__dirname, '..', '..', 'code');
export const PACKS_DIRECTORY = join(__dirname, '..', '..', 'packs');
export const REPROS_DIRECTORY = join(__dirname, '..', '..', 'repros');
export const SANDBOX_DIRECTORY = join(__dirname, '..', '..', 'sandbox');
export const JUNIT_DIRECTORY = join(__dirname, '..', '..', 'test-results');
Expand Down