Core: Don't persist theme to localStorage

[ndelangen]

Issue: #6798

What I did

The FOUC doesn't happen anymore because of the tri-config api change
I removed the code to prevent the FOUC which was causing other issues.

[vercel]

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/storybook/monorepo/o5s0b60t1
✅ Preview: https://monorepo-git-core-no-persist-theme.storybook.now.sh

[shilman]

AWESOME. Super excited about this!!!

What happens to users who haven't migrated to the new API?

[ndelangen]

They get the FOUC, seems like a good trade-off to me.

It will nudge them to migrate to the new api I guess.

[shilman]

Yeah I think that's a fair tradeoff and a great reason to upgrade.

Is there a deprecation warning for the old API, or some other clear path for them to follow when they get the FOUC?

[ndelangen]

@shilman if this is going to be in a major release, is it important enough to get a deprecation warning in?

I looked, and it's not the healthiest code already with multiple layers of deprecated stuff. I'd rather clean it up some more instead.

all this code is already for deprecation warnings:

storybook/lib/api/src/modules/layout.ts

Lines 86 to 131 in 0de8e9d

	const deprecationMessage = (optionsMap: OptionsMap, prefix = '') =>
	`The options { ${Object.keys(optionsMap).join(', ')} } are deprecated -- use ${
	prefix ? `${prefix}'s` : ''
	} { ${Object.values(optionsMap).join(', ')} } instead.`;
	const applyDeprecatedThemeOptions = deprecate(({ name, url, theme }: Options): PartialThemeVars => {
	const { brandTitle, brandUrl, brandImage }: PartialThemeVars = theme || {};
	return {
	brandTitle: brandTitle || name,
	brandUrl: brandUrl || url,
	brandImage: brandImage || null,
	};
	}, deprecationMessage(deprecatedThemeOptions));
	const applyDeprecatedLayoutOptions = deprecate((options: Options): PartialLayout => {
	const layoutUpdate: PartialLayout = {};
	['goFullScreen', 'showStoriesPanel', 'showAddonPanel'].forEach(
	(option: 'goFullScreen' | 'showStoriesPanel' | 'showAddonPanel') => {
	const v = options[option];
	if (typeof v !== 'undefined') {
	const key = deprecatedLayoutOptions[option];
	layoutUpdate[key] = v;
	}
	}
	);
	if (options.addonPanelInRight) {
	layoutUpdate.panelPosition = 'right';
	}
	return layoutUpdate;
	}, deprecationMessage(deprecatedLayoutOptions));
	const checkDeprecatedThemeOptions = (options: Options) => {
	if (Object.keys(deprecatedThemeOptions).find(v => v in options)) {
	return applyDeprecatedThemeOptions(options);
	}
	return {};
	};
	const checkDeprecatedLayoutOptions = (options: Options) => {
	if (Object.keys(deprecatedLayoutOptions).find(v => v in options)) {
	return applyDeprecatedLayoutOptions(options);
	}
	return {};
	};

[ndelangen]

Can I remove all that code for 6.0.0?

[shilman]

@ndelangen Yeah I think we can get rid of that old deprecation warning. If we do, we should mention it in MIGRATION.md for 6.0 and refer back to the original 5.0 notice. cc @tmeasday @Hypnosphi

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[shilman]

LGTM! Safe to merge?