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

[Web UI] Create separate vendor and lib DLL bundles #2822

Merged
merged 3 commits into from
Mar 22, 2019
Merged

Conversation

10xjs
Copy link
Contributor

@10xjs 10xjs commented Mar 21, 2019

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 to yarn build and re-run yarn start to see any changes in the browser. Depending on the scope of the changes, it may be possible to run yarn build:app or yarn 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.

10xjs added 2 commits March 21, 2019 12:28
Signed-off-by: Neal Granger <neal@nealg.com>
Signed-off-by: Neal Granger <neal@nealg.com>
@10xjs 10xjs requested a review from jamesdphillips March 21, 2019 21:04
@10xjs 10xjs added the component:web-ui Sensu dashboard improvements label Mar 21, 2019
Copy link
Contributor

@jamesdphillips jamesdphillips left a 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')}" />
Copy link
Contributor

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}>
Copy link
Contributor

@jamesdphillips jamesdphillips Mar 21, 2019

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.

Copy link
Contributor Author

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>
Copy link
Contributor

@jamesdphillips jamesdphillips left a comment

Choose a reason for hiding this comment

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

:shipit:

@10xjs 10xjs merged commit 44d00b3 into master Mar 22, 2019
@jamesdphillips jamesdphillips deleted the web/dll-build branch March 22, 2019 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:web-ui Sensu dashboard improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants