Vue3: Add support for Global Apps install by chakAs3 · Pull Request #23772 · storybookjs/storybook

chakAs3 · 2023-08-09T13:39:21Z

What I did

How to test

Checklist

Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
Make sure to add/update documentation regarding your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

kasperpeulen

Can you fill in "What I did" and "How to test"? @chakAs3

kasperpeulen · 2023-08-21T13:35:12Z

+    module.hot.decline();
+  }
+} catch (e) {
+  console.log('  e:', e);


Do we need to log this?

not need to log but we need to wrap it in try catch. , it will break in vite ( it is webpack specific )

Okay, let's remove the console.log then

kasperpeulen · 2023-08-21T13:36:05Z

+
+async function installGlobalPlugins(app: App<any>, storyContext: StoryContext<VueRenderer>) {
+  if (window.APPLY_PLUGINS_FUNC) {
+    await window.APPLY_PLUGINS_FUNC(app, storyContext);


What is the difference with the setup function? Should this function be set by the user?

using setup is not safe, in case your package manager allows 2 versions of the same package, @storybook/vue3 can't be a singleton, so calling setup. seems not working for users
using window. APPLY_PLUGINS_FUNC will insure the singleton.
it should be. awaited as well in case. registering the plugin is async

I think we can also solve this by putting this on the window:

storybook/code/renderers/vue3/src/render.ts

Line 20 in 936602d

const setupFunctions = new Set<(app: App, storyContext?: StoryContext<VueRenderer>) => void>();

I think it is better have one API.

yes exactly i'm working on that idea

kasperpeulen

Looks good! Some small stuff.

kasperpeulen · 2023-08-22T07:27:04Z

 // optimization: stop HMR propagation in webpack
-if (typeof module !== 'undefined') module?.hot?.decline();
+try {
+  if (module?.hot?.decline) {
+    module.hot.decline();
+  }


You still need the typeof module !== 'undefined' check right?

Maybe:

if (typeof module !== 'undefined') module?.hot?.decline?.();

kasperpeulen · 2023-08-22T07:29:24Z

+  globalThis.PLUGINS_SETUP_FUNCTIONS ??= new Set();
+  globalThis.PLUGINS_SETUP_FUNCTIONS.add(fn);


@ndelangen I thought you mentioned at some point that we prefer @storybook/global over globalThis but I don't know the reasons anymore.

as i know globalThis was introduced to give access to global scope regardless the system Node/Browser, it is consistency and standard rather than using window / global

and that what @storybook/global provides the global scope so if we do
declare var STORYBOOK_ENV:'type' in type.d.ts file
will be safely available on globalThis as global module

kasperpeulen · 2023-08-22T07:30:12Z

+  if (globalThis && globalThis.PLUGINS_SETUP_FUNCTIONS)
+    await Promise.all([...globalThis.PLUGINS_SETUP_FUNCTIONS].map((fn) => fn(app, storyContext)));


Maybe Promise.allSettled as I think we don't want to short-circuit here?

kasperpeulen · 2023-08-22T07:30:32Z

+const runSetupFunctions = async (
+  app: App,
+  storyContext: StoryContext<VueRenderer>
+): Promise<any> => {


Promise<void>

kasperpeulen

LGTM!

chakAs3 added 2 commits August 9, 2023 17:19

added global vue mixins, plugin, install

0a5ee9e

fix _a.decline not a function (if vite is the builder)

466850a

chakAs3 requested review from JReinhold and kasperpeulen as code owners August 9, 2023 13:39

chakAs3 added the vue3 label Aug 9, 2023

chakAs3 added 2 commits August 13, 2023 03:13

refactory and cleanup

e45779e

Merge branch 'next' into chaks/vue3-global-app-install

d0a16cc

kasperpeulen marked this pull request as draft August 14, 2023 07:29

chakAs3 added 2 commits August 14, 2023 16:27

remove NUXT to be more generic

1f23839

Merge branch 'next' into chaks/vue3-global-app-install

fe71e39

chakAs3 added the bug label Aug 21, 2023

chakAs3 assigned chakAs3 and unassigned chakAs3 Aug 21, 2023

Merge branch 'next' into chaks/vue3-global-app-install

886dfdf

chakAs3 marked this pull request as ready for review August 21, 2023 11:30

chakAs3 added the ci:normal Run our default set of CI jobs (choose this for most PRs). label Aug 21, 2023

chakAs3 mentioned this pull request Aug 21, 2023

[Bug]: setup function imported from '@storybook/vue3' is not working #23844

Closed

kasperpeulen reviewed Aug 21, 2023

View reviewed changes

Comment thread code/renderers/vue3/src/globals.ts Outdated

kasperpeulen reviewed Aug 21, 2023

View reviewed changes

kasperpeulen marked this pull request as draft August 21, 2023 13:41

chakAs3 added 2 commits August 21, 2023 18:06

refactor

69b9155

Merge branch 'next' into chaks/vue3-global-app-install

6dd671e

chakAs3 marked this pull request as ready for review August 21, 2023 14:07

JReinhold assigned JReinhold and kasperpeulen Aug 21, 2023

JReinhold marked this pull request as draft August 21, 2023 14:11

chakAs3 added 4 commits August 21, 2023 23:06

Merge branch 'next' into chaks/vue3-global-app-install

f3ae06f

refactor functions

be7d7a0

fix async iteration over Set

33d067c

no need to return

07de8db

chakAs3 marked this pull request as ready for review August 22, 2023 05:24

kasperpeulen requested changes Aug 22, 2023

View reviewed changes

cleanup code

6d53c68

kasperpeulen approved these changes Aug 23, 2023

View reviewed changes

kasperpeulen merged commit fd9aa21 into next Aug 23, 2023

kasperpeulen deleted the chaks/vue3-global-app-install branch August 23, 2023 08:32

This was referenced Aug 23, 2023

Release: Prerelease 7.4.0-alpha.1 #23865

Merged

Release: Prerelease 7.4.0-alpha.1 #23924

Closed

th1nkgr33n mentioned this pull request Aug 23, 2023

Nuxt i18n module support storybook-vue/storybook-nuxt#7

Closed

		globalThis.PLUGINS_SETUP_FUNCTIONS ??= new Set();
		globalThis.PLUGINS_SETUP_FUNCTIONS.add(fn);

		if (globalThis && globalThis.PLUGINS_SETUP_FUNCTIONS)
		await Promise.all([...globalThis.PLUGINS_SETUP_FUNCTIONS].map((fn) => fn(app, storyContext)));

Uh oh!

Conversation

chakAs3 commented Aug 9, 2023 • edited by vanessayuenn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

How to test

Checklist

Maintainers

🦋 Canary release

Uh oh!

Uh oh!

kasperpeulen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kasperpeulen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kasperpeulen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chakAs3 commented Aug 9, 2023 •

edited by vanessayuenn

Loading

kasperpeulen left a comment •

edited

Loading