-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Conversation
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
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/storybook/monorepo/4bs1g69i7 |
const firstProject = projects[Object.keys(projects)[0]]; | ||
return projects.storybook || fallbackProject || firstProject; | ||
let projectName; | ||
const firstProjectName = Object.keys(projects)[0]; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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'; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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:
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