Skip to content

build: set up generic way of passing in environment variables to dev app #21825

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

Merged
merged 1 commit into from
Feb 9, 2021
Merged

build: set up generic way of passing in environment variables to dev app #21825

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
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,6 @@ testem.log
*.log
.ng-dev.user*
.husky/_
/src/dev-app/google-maps-api-key.txt

# Variables that are inlined into the dev app index.html
/src/dev-app/variables.json
11 changes: 6 additions & 5 deletions src/dev-app/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,12 @@ create_system_config(
output_name = "system-config.js",
)

Copy link
Member Author

Choose a reason for hiding this comment

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

@josephperrott I don't know how to resolve the lint errors in this file. I get a lint failure even after I run yarn format and the error doesn't actually say what's wrong. Here's the full log:

yarn run v1.22.10
Checking format of 1 file(s)

The following files are out of format:
    - src/dev-app/BUILD.bazel

? Format the files now? Yes
Formatting 1 file(s)
[========================================] ETA: 0s | 1/1 files
√  Formatting complete.
Done in 5.56s.

Copy link
Member

Choose a reason for hiding this comment

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

This issue was actually just reported a few days ago in another case. I need to address the error messaging that occurs to surface it.

For the specific situation, you can have a glob without a matcher.

You should instead have

filegroup(
    name = "variables",
    srcs = [
        "variables.json"
    ],
)

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that the variables.json is an optional file that is .gitignored. Wouldn't using a filegroup like this result in an error if the file doesn't exist?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I ran into this problem too. You need the glob, because the file doesn't necessarily exist, but it needs some arbitrary matcher because of the lint issue, I assume due to the assumption that you would never need a glob for a single file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice, adding a * in there fixed it 👍

# File group for static files that are listed in the gitignore file that contain
# secrets like API keys.
# Variables that are going to be inlined into the dev app index.html.
filegroup(
name = "environment-secret-assets",
srcs = glob(["*-api-key.txt"]),
name = "variables",
# Note that we need the * in the pattern, because there's a lint rule which
# doesn't allow single files in a `glob`. We have to use a glob, because the file is optional.
srcs = glob(["*variables.json"]),
)

# File group for all static files which are needed to serve the dev-app. These files are
Expand All @@ -137,9 +138,9 @@ filegroup(
srcs = [
"favicon.ico",
"index.html",
":environment-secret-assets",
":system-config",
":theme",
":variables",
"//src/dev-app/icon:icon_demo_assets",
"//tools:system-rxjs-operators.js",
"@npm//:node_modules/@material/animation/dist/mdc.animation.js",
Expand Down
23 changes: 6 additions & 17 deletions src/dev-app/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,19 @@
<body>
<dev-app>Loading...</dev-app>

<!-- This iframe loads the hidden Google Maps API Key. -->
<iframe id="google-maps-api-key"
src="google-maps-api-key.txt"
style="display:none;"
onload="loadGoogleMapsScript()"></iframe>

<script src="core-js-bundle/index.js"></script>
<script src="zone.js/dist/zone.js"></script>
<script src="systemjs/dist/system.js"></script>
<script src="system-config.js"></script>
<script src="https://www.youtube.com/iframe_api"></script>
<script src="https://unpkg.com/@googlemaps/markerclustererplus/dist/index.min.js"></script>
<script>
function loadGoogleMapsScript() {
var iframe = document.getElementById('google-maps-api-key');
var googleMapsScript = document.createElement('script');
var googleMapsApiKey = iframe.contentDocument.body.textContent;
var googleMapsUrl = 'https://maps.googleapis.com/maps/api/js?libraries=visualization';
if (googleMapsApiKey !== 'Page not found') {
googleMapsUrl = googleMapsUrl + '&key=' + googleMapsApiKey;
}
googleMapsScript.src = googleMapsUrl;
document.body.appendChild(googleMapsScript);
}
(function loadGoogleMaps(key) {
var script = document.createElement('script');
script.src = 'https://maps.googleapis.com/maps/api/js?libraries=visualization' +
(key ? '&key=' + key : '');
document.body.appendChild(script);
})(window.DEV_APP_VARIABLES.GOOGLE_MAPS_KEY);
</script>
<script>
System.config({
Expand Down
49 changes: 37 additions & 12 deletions tools/dev-server/dev-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {readFileSync, existsSync} from 'fs';
import * as browserSync from 'browser-sync';
import * as http from 'http';
import * as path from 'path';
Expand All @@ -17,6 +18,9 @@ import * as send from 'send';
* environment and on Windows (with a runfile manifest file).
*/
export class DevServer {
/** Cached content of the index.html. */
private _index: string|null = null;

/** Instance of the browser-sync server. */
server = browserSync.create();

Expand All @@ -38,7 +42,7 @@ export class DevServer {
private _historyApiFallback: boolean = false) {}

/** Starts the server on the given port. */
async start() {
start() {
return new Promise<void>((resolve, reject) => {
this.server.init(this.options, (err) => {
if (err) {
Expand Down Expand Up @@ -82,30 +86,51 @@ export class DevServer {
// to the index: https://github.com/bripkens/connect-history-api-fallback#introduction
if (this._historyApiFallback && req.method === 'GET' && !req.url.includes('.') &&
req.headers.accept && req.headers.accept.includes('text/html')) {
req.url = '/index.html';
}
res.end(this._getIndex());
} else {
const resolvedPath = this._resolveUrlFromRunfiles(req.url);

const resolvedPath = this._resolveUrlFromRunfiles(req.url);
if (resolvedPath === null) {
res.statusCode = 404;
res.end('Page not found');
return;
}

if (resolvedPath === null) {
res.statusCode = 404;
res.end('Page not found');
return;
send(req, resolvedPath).pipe(res);
}

send(req, resolvedPath).pipe(res);
}

/** Resolves a given URL from the runfiles using the corresponding manifest path. */
private _resolveUrlFromRunfiles(url: string): string|null {
for (let rootPath of this._rootPaths) {
try {
return require.resolve(path.posix.join(rootPath, getManifestPath(url)));
} catch {
}
} catch {}
}
return null;
}

/** Gets the content of the index.html. */
private _getIndex(): string {
if (!this._index) {
const indexPath = this._resolveUrlFromRunfiles('/index.html');

if (!indexPath) {
throw Error('Could not resolve dev server index.html');
}

// We support specifying a variables.json file next to the index.html which will be inlined
// into the dev app as a `script` tag. It is used to pass in environment-specific variables.
const varsPath = path.join(path.dirname(indexPath), 'variables.json');
const scriptTag = '<script>window.DEV_APP_VARIABLES = ' +
(existsSync(varsPath) ? readFileSync(varsPath, 'utf8') : '{}') + ';</script>';
const content = readFileSync(indexPath, 'utf8');
const headIndex = content.indexOf('</head>');
this._index = content.slice(0, headIndex) + scriptTag + content.slice(headIndex);
}

return this._index;
}
}

/** Gets the manifest path for a given url */
Expand Down