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

fix: 156 obsolete browsers #159

Merged
merged 2 commits into from
Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions client/configuration/webpack/SUPPORTED_BROWSERS_REGEX.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions client/configuration/webpack/obsoleteBrowsers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* eslint-disable @blueprintjs/classes-constants -- do not import files */
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this too will need to be included in CSP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see comment above!

const dataset =
typeof document !== "undefined" &&
document.querySelector("#obsolete-browsers").dataset;

const datasetRegex = dataset.regex;

const regex = new RegExp(datasetRegex.slice(1, -1));

if (dataset && !regex.test(navigator.userAgent)) {
const root = document.getElementById("root");
root.remove();
const portals = document.getElementsByClassName("bp3-portal");
for (let i = 0; i < portals.length; i += 1) {
portals[i].remove();
}

const element = document.createElement("div");
element.innerHTML = dataset.template;
document.body.appendChild(element);
}
/* eslint-enable @blueprintjs/classes-constants -- do not import files */
9 changes: 0 additions & 9 deletions client/configuration/webpack/obsoleteHTMLTemplate.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
<script>
/** Reflect ANY changes to this script in the script hash in `server/eb/app.py` **/
var root = document.getElementById("root");
root.remove();
var portals = document.getElementsByClassName("bp3-portal");
for (var i = 0; i < portals.length; i += 1) {
portals[i].remove();
}
</script>
Comment on lines -1 to -9
Copy link
Contributor

Choose a reason for hiding this comment

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

We're going to need to update CSP

Copy link
Contributor Author

@tihuan tihuan Oct 19, 2021

Choose a reason for hiding this comment

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

Ah thank you! Yeah we can remove the associated hash now! I'll add it as a TODO in the PR and ticket descriptions 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OH! It's just in the BE code not in Infra repo lol Updated! Thank you!

<div
style="
display: flex;
Expand Down
5 changes: 0 additions & 5 deletions client/configuration/webpack/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ const HtmlWebpackPlugin = require("html-webpack-plugin");
// eslint-disable-next-line @typescript-eslint/no-var-requires --- FIXME: disabled temporarily on migrate to TS.
const FaviconsWebpackPlugin = require("favicons-webpack-plugin");
// eslint-disable-next-line @typescript-eslint/no-var-requires --- FIXME: disabled temporarily on migrate to TS.
const ScriptExtHtmlWebpackPlugin = require("script-ext-html-webpack-plugin");
// eslint-disable-next-line @typescript-eslint/no-var-requires --- FIXME: disabled temporarily on migrate to TS.
const MiniCssExtractPlugin = require("mini-css-extract-plugin");

// eslint-disable-next-line @typescript-eslint/no-var-requires --- FIXME: disabled temporarily on migrate to TS.
Expand Down Expand Up @@ -83,9 +81,6 @@ const devConfig = {
CXG_SERVER_PORT: process.env.CXG_SERVER_PORT || "5005",
}),
}),
new ScriptExtHtmlWebpackPlugin({
async: "obsolete",
}),
],
infrastructureLogging: {
level: "warn",
Expand Down
24 changes: 13 additions & 11 deletions client/configuration/webpack/webpack.config.shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
const path = require("path");
const fs = require("fs");
const MiniCssExtractPlugin = require("mini-css-extract-plugin");
const ObsoleteWebpackPlugin = require("obsolete-webpack-plugin");
const ScriptExtHtmlWebpackPlugin = require("script-ext-html-webpack-plugin");
const webpack = require("webpack");
const CopyPlugin = require("copy-webpack-plugin");
const SUPPORTED_BROWSERS_REGEX = require("./SUPPORTED_BROWSERS_REGEX");

const src = path.resolve("src");
const nodeModules = path.resolve("node_modules");
Expand All @@ -16,7 +16,7 @@ const rawObsoleteHTMLTemplate = fs.readFileSync(
"utf8"
);

const obsoleteHTMLTemplate = rawObsoleteHTMLTemplate.replace(/'/g, '"');
const obsoleteHTMLTemplate = rawObsoleteHTMLTemplate.replace(/"/g, "'");

const deploymentStage = process.env.DEPLOYMENT_STAGE || "test";

Expand Down Expand Up @@ -68,21 +68,23 @@ module.exports = {
],
},
plugins: [
new ObsoleteWebpackPlugin({
name: "obsolete",
template: obsoleteHTMLTemplate,
promptOnNonTargetBrowser: false,
}),
new ScriptExtHtmlWebpackPlugin({
async: "obsolete",
}),
new webpack.DefinePlugin({
OBSOLETE_TEMPLATE: JSON.stringify(obsoleteHTMLTemplate),
OBSOLETE_REGEX: JSON.stringify(String(SUPPORTED_BROWSERS_REGEX)),
PLAUSIBLE_DATA_DOMAIN: JSON.stringify(
deploymentStage === "prod"
? "cellxgene.cziscience.com"
: "cellxgene.staging.single-cell.czi.technology"
),
}),
new CopyPlugin({
patterns: [
{
from: "configuration/webpack/obsoleteBrowsers.js",
to: "static/obsoleteBrowsers.js",
},
],
}),
],
};
/* eslint-enable @blueprintjs/classes-constants -- we don't import blueprint here */
7 changes: 7 additions & 0 deletions client/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,12 @@
>

<div id="root"></div>
<script
async
src="static/obsoleteBrowsers.js"
id="obsolete-browsers"
data-template="<%= OBSOLETE_TEMPLATE %>"
data-regex="<%= OBSOLETE_REGEX %>"
></script>
Comment on lines +45 to +51
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this going to need to be reflected in CSP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great question! Since the script will be served from the same domain as the HTML page, script-src "'self'" should cover it. Only inline scripts need hashes 👍

</body>
</html>
7 changes: 7 additions & 0 deletions client/index_template.html
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@
</noscript>

<div id="root"></div>
<script
async
src="static/obsoleteBrowsers.js"
id="obsolete-browsers"
data-template="<%= OBSOLETE_TEMPLATE %>"
data-regex="<%= OBSOLETE_REGEX %>"
></script>
{% for script in SCRIPTS -%}
<script type="text/javascript"
{{ ('integrity="%s"' % script.integrity) | safe if script.integrity }}
Expand Down
Loading