-
Notifications
You must be signed in to change notification settings - Fork 176
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
[Web UI] Create separate vendor and lib DLL bundles #2822
Conversation
Signed-off-by: Neal Granger <neal@nealg.com>
Signed-off-by: Neal Granger <neal@nealg.com>
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.
Noticed one small regression but Webpack config looks v good. 👍
<link rel="manifest" href="${require('./manifest.macro.js')}"> | ||
|
||
<!-- Windows 8.1 + IE11 and above --> | ||
<meta name="msapplication-config" content="${require('./browserconfig.macro.js')}" /> |
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.
neato
@@ -115,20 +116,18 @@ class SilencesListItem extends React.Component { | |||
paddingTop: 8, // one spacing unit | |||
}} | |||
> | |||
<Maybe value={silence.creator}> |
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.
I think this may be a regression that was introduced in the rebase. See: #2817.
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.
good catch! This is one of two conflicts I had to resolve
Signed-off-by: Neal Granger <neal@nealg.com>
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 is this change?
This change breaks the single Web UI client bundle into multiple bundles.
Core components and shared dependencies are provided by "lib" and "vendor" bundles that can be referenced by downstream webpack builds allowing them to add additional functionality to the core Web UI app.
Why is this change necessary?
This changes enables the creation of the Sensu Enterprise Web UI. (https://github.com/sensu/sensu-enterprise-go/issues/229)
Does your change need a Changelog entry?
Yes.
Do you need clarification on anything?
No.
Were there any complications while making this change?
This feature required major changes of the Web UI codebase and build directory structures.
The dashboard assets go generate logic does not support the changes to the build directory structure - regenerated assets are intentionally omitted from this PR to avoid breaking things.
The
yarn start
script no longer runs a build and does not reload changes while the server is running. It is necessary toyarn build
and re-runyarn start
to see any changes in the browser. Depending on the scope of the changes, it may be possible to runyarn build:app
oryarn build:lib && yarn build:app
to speed things up.Have you reviewed and updated the documentation for this change? Is new documentation required?
No,
How did you verify this change?
By running
yarn test
and by manual Web UI testing.