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

Set other manager-side constants in build #18728

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

tmeasday
Copy link
Member

Issue: DOCS_MODE (and others) weren't getting set.

What I did

Pass options to the htmlTemplate() function and use for these options

How to test

  • Check release notes
  • Run with --docs
  • etc

@tmeasday tmeasday requested a review from ndelangen July 18, 2022 00:04
@nx-cloud
Copy link

nx-cloud bot commented Jul 18, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit c57358f. 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.

@@ -72,6 +73,12 @@ export const renderHTML = async (
FEATURES: JSON.stringify(await features, null, 2),
Copy link
Member Author

Choose a reason for hiding this comment

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

We seem to apply('features') and await it several times unnecessarily throughout the code base, as we already do this:

const features = await presets.apply<StorybookConfig['features']>('features');
global.FEATURES = features;
const fullOptions: Options = {
...options,
presets,
features,
};

The same may be true for logLevel and refs, I'm not sure. I don't pretend to understand this code well enough to say.

Copy link
Member

Choose a reason for hiding this comment

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

Evaluating the preset value once, and passing down the value might increase perf by a tiny bit, but at the cost of complexity.

Apply-ing a preset multiple times, is the intended behavior. But what we should do, at some point, is to enable memoization on it, so calling it multiple times with the same options, gets cached.

@ndelangen ndelangen merged commit 58acdd0 into future/base Jul 18, 2022
@ndelangen ndelangen deleted the export-other-constants-for-manager branch July 18, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants