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

Future / pre-built manager - using esbuild #18550

Merged
merged 77 commits into from
Jul 6, 2022
Merged

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jun 23, 2022

What I did

This is looking promising!

  • create a new package called @storybook/builder-manager
  • change bundling of lib/ui to bundle everything using tsup
  • add an entry-point to lib/ui called runtime, which is self-invoking
  • I add the prebundled runtime to the main template as script reference.
  • in the new manager builder, proxy the dist of lib/ui as-is including the runtime
  • in the new manager builder, run all managerEntries through esbuild
  • ensure that all the shared dependencies between addons and lib/ui is globalized & replaced
  • delete the manager-webpack5 package
  • delete code in core-common relating to the manager build

- We prebundle the entire lib/ui with tsup.
- We tell the manager builder to serve lib/ui/dist as part of staticDirs
- We add the prebundled runtime to the main template as script reference

This works! Webpack builds ONLY the addons, not the manager UI!

But there's a problem... the manager gets react components injected into it.
These react components do not share the same version of react.
That causes the well know: hooks-call problems.

So left to investigate:
- Will this really work? Does bundling lib/ui and everything it needs break nothing?
- How to force a single version of react? (all dependencies normally coming from the manager, really)
@nx-cloud
Copy link

nx-cloud bot commented Jun 23, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 8a703bf. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@ndelangen ndelangen changed the base branch from next to future/tsup-libs-1 June 23, 2022 14:45
@ndelangen ndelangen added the maintenance User-facing maintenance tasks label Jun 24, 2022
@ndelangen ndelangen self-assigned this Jun 24, 2022
@ndelangen ndelangen requested review from a team, shilman and tmeasday June 24, 2022 14:59
@ndelangen
Copy link
Member Author

I will delete the old manager-webpack5 package, probably

# Conflicts:
#	lib/manager-webpack5/src/presets/manager-preset.ts
…into future/prebuilt-manager-esbuild

# Conflicts:
#	addons/storyshots/storyshots-puppeteer/package.json
#	yarn.lock
# Conflicts:
#	addons/essentials/package.json
#	frameworks/html-webpack5/package.json
#	frameworks/preact-webpack5/package.json
#	frameworks/react-webpack5/package.json
#	frameworks/server-webpack5/package.json
#	frameworks/svelte-webpack5/package.json
#	frameworks/vue-webpack5/package.json
#	frameworks/vue3-webpack5/package.json
#	frameworks/web-components-webpack5/package.json
#	lib/blocks/package.json
#	lib/cli/src/versions.ts
#	lib/components/package.json
#	lib/core-common/package.json
#	lib/core-server/package.json
#	lib/core-server/src/utils/open-in-browser.ts
#	lib/manager-webpack5/package.json
#	lib/manager-webpack5/typings.d.ts
#	lib/theming/package.json
#	lib/ui/package.json
#	presets/html-webpack/package.json
#	presets/preact-webpack/package.json
#	presets/react-webpack/package.json
#	presets/server-webpack/package.json
#	presets/svelte-webpack/package.json
#	presets/vue-webpack/package.json
#	presets/vue3-webpack/package.json
#	presets/web-components-webpack/package.json
#	renderers/html/package.json
#	renderers/preact/package.json
#	renderers/react/package.json
#	renderers/server/package.json
#	renderers/svelte/package.json
#	renderers/vue/package.json
#	renderers/vue3/package.json
#	renderers/web-components/package.json
#	yarn.lock
add support for setting config after manager has loaded
@ndelangen
Copy link
Member Author

Refs and async configs are in.

Theme flashing isn't an issue!

@ndelangen
Copy link
Member Author

And it's ready for review, should be low-noise now

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Looks good to me! A couple of suggestions for improvement, but generally speaking it's very nice!

lib/api/src/modules/layout.ts Outdated Show resolved Hide resolved
lib/cli/src/automigrate/fixes/webpack5.ts Show resolved Hide resolved
lib/ui/src/globals/exports.ts Show resolved Hide resolved
lib/ui/src/globals/definitions.ts Show resolved Hide resolved
@ndelangen ndelangen merged commit 34a13e9 into future/base Jul 6, 2022
@ndelangen ndelangen deleted the future/pbm/stage0 branch July 6, 2022 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants