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

Jump to bottom

refactor: Remove liquid templates #353

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

refactor: Remove liquid templates #353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
370b48d
remove liquidjs-loader
abovedave Oct 1, 2024
abe4726
implement simple html templates
abovedave Oct 1, 2024
68c162c
remove String()
abovedave Oct 1, 2024
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
39 changes: 16 additions & 23 deletions config/webpack.dev.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ const common = require("./webpack.common.js");
const HtmlWebpackPlugin = require("html-webpack-plugin");
const fs = require("fs");

const htmlLayout = fs.readFileSync("./site/layout.html");
Copy link
Collaborator

Choose a reason for hiding this comment

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

optional: Move this into the exported function (after line 8) so it only runs when invoked.

In this circumstance it changes basically nothing, but its generally good form to avoid side effects on import when possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

loader-update: Set the "utf-8" flag so the file is read as a string (instead of Buffer)

Suggested change
const htmlLayout = fs.readFileSync("./site/layout.html");
const htmlLayout = fs.readFileSync("./site/layout.html", "utf-8");


module.exports = (env, argv) => {
// add --mode=production to flip this into a pseudo-production server
const emulateProdServer = argv.mode === "production";
Expand All @@ -17,22 +19,6 @@ module.exports = (env, argv) => {
},
mode: emulateProdServer ? "production" : "development",
devtool: emulateProdServer ? false : "inline-source-map",
module: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

loader-update: Partially revert this change, modifying html-loader to do the work for us (GitHub doesn't allow suggestions on deleted lines, so you'll have to fix this manually)

            module: {
                rules: [
                    {
                        test: /\.html$/,
                        use: [
                            {
                                loader: "html-loader",
                                options: {
                                    // wrap the content of the html file in the layout.html file
                                    preprocessor: (content) =>
                                        htmlLayout.replace(
                                            "<%=content%>",
                                            content
                                        ),
                                },
                            },
                        ],
                    },
                ],
            },

rules: [
{
test: /\.html$/,
use: [
"html-loader",
{
loader: "liquidjs-loader",
options: {
root: "./site/",
},
},
],
},
],
},
devServer: {
open: false,
host:
Expand All @@ -51,13 +37,20 @@ module.exports = (env, argv) => {
},
plugins: [
// create an html page for every item in ./site/views
...fs.readdirSync("./site/views").map(
(f) =>
new HtmlWebpackPlugin({
template: "./site/views/" + f,
filename: f,
})
),
...fs.readdirSync("./site/views").map((f) => {
const htmlView = fs.readFileSync(
`./site/views/${f}`,
"utf8"
);

return new HtmlWebpackPlugin({
templateContent: new Function(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The use of new Function here is a large code smell to me. I'll pull down the PR here shortly and see if I can't whip up an alternative approach.

["content"],
`return \`${htmlLayout}\`;`
)(htmlView),
filename: f,
});
}),
Comment on lines +40 to +53
Copy link
Collaborator

Choose a reason for hiding this comment

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

loader-update: Revert this change

Suggested change
...fs.readdirSync("./site/views").map((f) => {
const htmlView = fs.readFileSync(
`./site/views/${f}`,
"utf8"
);
return new HtmlWebpackPlugin({
templateContent: new Function(
["content"],
`return \`${htmlLayout}\`;`
)(htmlView),
filename: f,
});
}),
...fs.readdirSync("./site/views").map(
(f) =>
new HtmlWebpackPlugin({
template: "./site/views/" + f,
filename: f,
})
),

],
optimization: {
splitChunks: {
Expand Down
99 changes: 0 additions & 99 deletions package-lock.json
View file
Open in desktop

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

1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
"husky": "^9.0.11",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"liquidjs-loader": "^1.0.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

🎉

"mini-css-extract-plugin": "^2.8.1",
"mini-svg-data-uri": "^1.4.4",
"postcss": "^8.4.38",
Expand Down
6 changes: 1 addition & 5 deletions site/layout.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,7 @@
<label id="a11y-editor-label" class="v-visible-sr">
Stacks-Editor demo implementation
</label>
{% block content %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll miss having this default example for reference, but... not so much that I'd block this change from going in.

lgtm

<h1 class="mt32">Minimum viable example</h1>
<textarea id="content" name="content" class="d-none"></textarea>
<div id="example-1"></div>
{% endblock %}
${content}
Copy link
Collaborator

Choose a reason for hiding this comment

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

loader-update: I changed this to use lodash templating syntax, which is what html-webpack-template uses by default. The syntax doesn't actually matter, but it'll at least be internally consistent if we decide to use any of the other supported templating features later.

Suggested change
${content}
<%=content%>

</main>
</body>
</html>
2 changes: 0 additions & 2 deletions site/views/dualeditor.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{% layout "layout.html" %} {% block content %}
<label id="a11y-editor-label-2" class="v-visible-sr">
Stacks-Editor second demo implementation
</label>
Expand All @@ -8,4 +7,3 @@ <h1 class="mt32">Dual Editor Playground</h1>
<div id="example-1" class="mb8 js-tables-enabled"></div>
<div id="example-2" class="js-tables-enabled"></div>
</div>
{% endblock %}
2 changes: 0 additions & 2 deletions site/views/empty.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
{% layout "layout.html" %} {% block content %}
<textarea id="content" name="content" class="d-none"></textarea>
<h1 class="mt32">Empty starter page</h1>
<div id="example-1" class="js-tables-enabled"></div>
{% endblock %}
2 changes: 0 additions & 2 deletions site/views/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{% layout "layout.html" %} {% block content %}
<textarea id="content" class="d-none">
# Here’s a thought

Expand Down Expand Up @@ -116,4 +115,3 @@
<h1>Stacks Editor</h1>
<h3>Showcasing our new editing experience for Stack Overflow</h3>
<div id="example-1"></div>
{% endblock %}
2 changes: 0 additions & 2 deletions site/views/md-preview.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{% layout "layout.html" %} {% block content %}
<textarea id="content" name="content" class="d-none">
# Here’s a thought

Expand Down Expand Up @@ -115,4 +114,3 @@
</textarea>
<h1 class="mt32">Markdown Preview Enabled</h1>
<div id="example-1" class="js-md-preview-enabled"></div>
{% endblock %}
2 changes: 0 additions & 2 deletions site/views/noimage.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
{% layout "layout.html" %} {% block content %}
<textarea id="content" name="content" class="d-none"></textarea>
<h1 class="mt32">this text area should populate without an image button</h1>
<div id="example-1" class="js-images-disabled"></div>
{% endblock %}
2 changes: 0 additions & 2 deletions site/views/plugins.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{% layout "layout.html" %} {% block content %}
<script src="https://cdn.jsdelivr.net/npm/mermaid/dist/mermaid.min.js"></script>

<textarea id="content" class="d-none">
Expand Down Expand Up @@ -31,4 +30,3 @@

<h1>Plugin Playground</h1>
<div id="example-1" class="js-plugins-enabled"></div>
{% endblock %}
Loading