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

Angular: Fix missing architect properties #9390

Merged
merged 2 commits into from Jan 14, 2020

Conversation

kroeder
Copy link
Member

@kroeder kroeder commented Jan 11, 2020

Issue: #9319

What I did

Improving dev-server logger outuputs

I added additional log outputs during the dev-server start. This should hopefully improve bug reports.

Starting storybook now looks like this:

info @storybook/angular v5.2.8
info
info => Loading presets
info => Loading presets
info => Loading custom addons config.
info => Found custom tsconfig.json
info => Loading custom Webpack config (full-control mode).
info => Using angular project 'my-awesome-project' for configuring Storybook. <----- this is new
info => Loading angular-cli config.
Browserslist: caniuse-lite is outdated. Please run next command `npm update`
info => Get angular-cli webpack config.
Starting type checking service...
Using 1 worker with 2048MB memory limit
webpack built 4a8afdd8c6e25eeeb15f in 20870ms
...

There are also new log outputs for errors.

Added missing architect.buildOptions

This fixes the referenced issue. Angular internally needs them and don't has default values for it so we must provide them.

How to test

  • Create angular project
  • Remove "styles", "scripts" or "outputPath"
  • Storybook should still start without any errors

this should help improving the error reporting when a user has trouble using app/angular
previously angular-devkit has thrown an error if they were missing

fixes #9319
@vercel
Copy link

vercel bot commented Jan 11, 2020

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/4bs1g69i7
✅ Preview: https://monorepo-git-angular-fix-missing-architect-properties.storybook.now.sh

const firstProject = projects[Object.keys(projects)[0]];
return projects.storybook || fallbackProject || firstProject;
let projectName;
const firstProjectName = Object.keys(projects)[0];
Copy link
Member

Choose a reason for hiding this comment

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

Is projects guaranteed to have at least one key?

Copy link
Member Author

Choose a reason for hiding this comment

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

It must, or else we are speaking of an empty project.
A couple of lines earlier there's also a check for making sure there is at least one project

if (!projects || !Object.keys(projects).length) {

@@ -90,6 +108,9 @@ export function getAngularCliWebpackConfigOptions(dirToSearch: Path) {
const tsConfigPath = path.resolve(dirToSearch, projectOptions.tsConfig) as Path;
const tsConfig = getTsConfigOptions(tsConfigPath);
const budgets = projectOptions.budgets || [];
const scripts = projectOptions.scripts || [];
const outputPath = projectOptions.outputPath || 'dist/storybook-angular';
Copy link
Member

Choose a reason for hiding this comment

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

what's the output path?

Copy link
Member Author

Choose a reason for hiding this comment

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

outputPath is needed by angular.json - more specifically it is needed by the function we call in @angular-devkit for receiving the default angular webpack config.

Storybook does not need it and usually it is already set in angular.json. If it is not set then storybook throws an error that it can't load the angular cli configuration

@shilman shilman changed the title Fix missing architect properties Angular: Fix missing architect properties Jan 12, 2020
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM!

@shilman shilman added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Jan 14, 2020
@shilman shilman merged commit 287a3a7 into next Jan 14, 2020
@shilman shilman deleted the angular-fix-missing-architect-properties branch January 14, 2020 08:28
@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
angular bug patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants