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.

Sign up for GitHub

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

ui: Exclude all debug-like files from the build #11211

Merged
merged 1 commit into from
Oct 8, 2021
Merged

ui: Exclude all debug-like files from the build #11211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions ui/packages/consul-acls/vendor/consul-acls/routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
(function(data) {
const appNameJS = data.appName.split('-')
.map((item, i) => i ? `${item.substr(0, 1).toUpperCase()}${item.substr(1)}` : item)
.join('');
data[`${appNameJS}Routes`] = JSON.stringify({
dc: {
acls: {
tokens: {
_options: {
abilities: ['read tokens'],
},
(routes => routes({
dc: {
acls: {
tokens: {
_options: {
abilities: ['read tokens'],
},
},
},
});
})(document.currentScript.dataset);
},
}))(
(json, data = document.currentScript.dataset) => {
const appNameJS = data.appName.split('-')
.map((item, i) => i ? `${item.substr(0, 1).toUpperCase()}${item.substr(1)}` : item)
.join('');
data[`${appNameJS}Routes`] = JSON.stringify(json);
}
);
64 changes: 33 additions & 31 deletions ui/packages/consul-partitions/vendor/consul-partitions/routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,38 @@
(function(data) {
const appNameJS = data.appName.split('-')
.map((item, i) => i ? `${item.substr(0, 1).toUpperCase()}${item.substr(1)}` : item)
.join('');
data[`${appNameJS}Routes`] = JSON.stringify({
dc: {
partitions: {
_options: {
path: '/partitions',
queryParams: {
sortBy: 'sort',
searchproperty: {
as: 'searchproperty',
empty: [['Name', 'Description']],
},
search: {
as: 'filter',
replace: true,
},
(routes => routes({
dc: {
partitions: {
_options: {
path: '/partitions',
queryParams: {
sortBy: 'sort',
searchproperty: {
as: 'searchproperty',
empty: [['Name', 'Description']],
},
abilities: ['read partitions'],
},
edit: {
_options: { path: '/:name' },
},
create: {
_options: {
template: 'dc/partitions/edit',
path: '/create',
abilities: ['create partitions'],
search: {
as: 'filter',
replace: true,
},
},
abilities: ['read partitions'],
},
edit: {
_options: { path: '/:name' },
},
create: {
_options: {
template: 'dc/partitions/edit',
path: '/create',
abilities: ['create partitions'],
},
},
},
});
})(document.currentScript.dataset);
},
}))(
(json, data = document.currentScript.dataset) => {
const appNameJS = data.appName.split('-')
.map((item, i) => i ? `${item.substr(0, 1).toUpperCase()}${item.substr(1)}` : item)
.join('');
data[`${appNameJS}Routes`] = JSON.stringify(json);
}
);
18 changes: 15 additions & 3 deletions ui/packages/consul-ui/app/instance-initializers/container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,21 @@
import { runInDebug } from '@ember/debug';
import require from 'require';
import merge from 'deepmerge';

import services from 'consul-ui/services';
import servicesDebug from 'consul-ui/services-debug';
const doc = document;
const appName = 'consul-ui';
const appNameJS = appName
.split('-')
.map((item, i) => (i ? `${item.substr(0, 1).toUpperCase()}${item.substr(1)}` : item))
.join('');
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocking comment/question, but curious to know if there are advantages to hand-rolling this as opposed to using something like @ember/string/classify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose the main reason is this file gets sourced/exec'd before ember is even loaded, and it's in a separate file (that is very very small and quick to load). So we don't have any ember things available to use at this point, and loading anything ember related in this file would bloat it out quite a bit I think.

I thought it might be worth briefly going over some of the objectives here as an FYI:

  • Whether this file gets sourced or not should be entirely controllable, dynamically, from The Outside. Pretty much in our case this means from the backend (whether that is Consul in prod or ember-cli in dev) via the index.html file (which the backend can control)
  • We should be able to safely retrieve the data in this file from our javascript app (our/Ember's service container) without having to change that file. Control as to what services get loaded should be entirely the concern of the backend.
  • This file shouldn't pollute the global scope with variables and/or functions in enabling our/Ember's service container to be able to retrieve this data.
  • Ideally this file would be just JSON (or potentially something less verbose), but we need the tiniest bit of code to convert this JSON to a string and put it somewhere accessible to our/Ember's service container.

Right now we 'safely' move this data from this file into our/Ember's service container using the app name to namespace the script tags that get 'slurped' up by the our/Ember's service container (to ensure we only slurp the correct data for the app). I'm unsure right now if doing this extra detail/level of 'safety' is complete overkill, and its the reason for this bit of code. So it's highly likely that we'll stop doing this by the time this hits a proper release. We could also just hardcode the consulUi string that this produces, hardcoding here wouldn't be a major issue at all either. All in all I've not decided yet, would be good to hear what you thoughts there are also tho.


export const services = merge.all(
[].concat(
...[...doc.querySelectorAll(`script[data-${appName}-services]`)].map($item =>
JSON.parse($item.dataset[`${appNameJS}Services`])
)
)
);

const inject = function(container, obj) {
// inject all the things
Expand All @@ -23,7 +36,6 @@ export default {
initialize(application) {

inject(application, services);
runInDebug(_ => inject(application, servicesDebug));

const container = application.lookup('service:container');
// find all the services and add their classes to the container so we can
Expand Down
8 changes: 0 additions & 8 deletions ui/packages/consul-ui/app/services-debug.js
View file
Open in desktop

This file was deleted.

11 changes: 10 additions & 1 deletion ui/packages/consul-ui/ember-cli-build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ module.exports = function(defaults, $ = process.env) {
$ = utils.env($);
const env = EmberApp.env();
const prodlike = ['production', 'staging'];
const devlike = ['development', 'staging'];
const sourcemaps = !['production'].includes(env) && !$('BABEL_DISABLE_SOURCEMAPS', false);

const trees = {};
Expand Down Expand Up @@ -46,6 +47,7 @@ module.exports = function(defaults, $ = process.env) {
// exclude any component/pageobject.js files from anything but test
excludeFiles = excludeFiles.concat([
'components/**/pageobject.js',
'components/**/test-support.js',
'components/**/*.test-support.js',
'components/**/*.test.js',
])
Expand All @@ -55,6 +57,8 @@ module.exports = function(defaults, $ = process.env) {
// exclude our debug initializer, route and template
excludeFiles = excludeFiles.concat([
'instance-initializers/debug.js',
'routing/**/*-debug.js',
'services/**/*-debug.js',
'templates/debug.hbs',
'components/debug/**/*.*'
])
Expand Down Expand Up @@ -83,7 +87,7 @@ module.exports = function(defaults, $ = process.env) {
trees.app = mergeTrees([
new Funnel('app', { exclude: excludeFiles })
].concat(
apps.filter(item => exists(`${item.path}/app`)).map(item => new Funnel(`${item.path}/app`))
apps.filter(item => exists(`${item.path}/app`)).map(item => new Funnel(`${item.path}/app`, {exclude: excludeFiles}))
), {
overwrite: true
});
Expand Down Expand Up @@ -144,6 +148,11 @@ module.exports = function(defaults, $ = process.env) {
outputFile: `assets/${item.name}/routes.js`,
});
});
['consul-ui/services'].concat(devlike ? ['consul-ui/services-debug'] : []).forEach(item => {
app.import(`vendor/${item}.js`, {
outputFile: `assets/${item}.js`,
});
});
// Use `app.import` to add additional libraries to the generated
// output files.
//
Expand Down
6 changes: 6 additions & 0 deletions ui/packages/consul-ui/lib/startup/templates/body.html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ ${environment === 'production' ? `{{jsonEncode .}}` : JSON.stringify(config.oper
"codemirror/mode/yaml/yaml.js": "${rootURL}assets/codemirror/mode/yaml/yaml.js"
}
</script>
<script data-app-name="${appName}" data-${appName}-services src="${rootURL}assets/consul-ui/services.js"></script>
${
environment === 'development' || environment === 'staging'
? `
<script data-app-name="${appName}" data-${appName}-services src="${rootURL}assets/consul-ui/services-debug.js"></script>
` : ``}
${
environment === 'production'
? `
Expand Down
15 changes: 15 additions & 0 deletions ui/packages/consul-ui/vendor/consul-ui/services-debug.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
(services => services({
"route:application": {
"class": "consul-ui/routing/application-debug"
},
"service:intl": {
"class": "consul-ui/services/i18n-debug"
}
}))(
(json, data = document.currentScript.dataset) => {
const appNameJS = data.appName.split('-')
.map((item, i) => i ? `${item.substr(0, 1).toUpperCase()}${item.substr(1)}` : item)
.join('');
data[`${appNameJS}Services`] = JSON.stringify(json);
}
);
11 changes: 9 additions & 2 deletions ui/packages/consul-ui/app/services.js → ...es/consul-ui/vendor/consul-ui/services.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export default {
(services => services({
"route:basic": {
"class": "consul-ui/routing/route"
},
Expand All @@ -11,4 +11,11 @@ export default {
"auth-provider:oidc-with-url": {
"class": "consul-ui/services/auth-providers/oauth2-code-with-url-provider"
}
}
}))(
(json, data = document.currentScript.dataset) => {
const appNameJS = data.appName.split('-')
.map((item, i) => i ? `${item.substr(0, 1).toUpperCase()}${item.substr(1)}` : item)
.join('');
data[`${appNameJS}Services`] = JSON.stringify(json);
}
);