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 frontend assets being outdated (Lombiq Technologies: OCORE-158) #15735

Merged
merged 75 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
aa7302e
Making assets_validation run for PRs and pushes too
Piedone Apr 11, 2024
ec68915
Updating TheAdmin CSS and trumbowyg-plugin webroot assets
Piedone Apr 11, 2024
d5f0a3f
Fixing concurrency group name
Piedone Apr 11, 2024
9b68682
Adding more trumbowyg-plugins changes
Piedone Apr 11, 2024
4b05e35
Trying standardized line endings for websoor folders
Piedone Apr 11, 2024
f35fc8a
Making buidling trumbowyg-plugins.js deterministic
Piedone Apr 11, 2024
1af2dd4
Adding nondeterministic Gulp buils warning to docs
Piedone Apr 11, 2024
9da0cc9
Adding more output on changed files
Piedone Apr 11, 2024
1bc54f7
Temporarily removing concurrency
Piedone Apr 11, 2024
4d08c57
Revert "Trying standardized line endings for websoor folders"
Piedone Apr 11, 2024
a86ec9a
Running the documented gulp rebuild instead
Piedone Apr 11, 2024
81bedb6
Debug change
Piedone Apr 11, 2024
dba85b6
Fixing trumbowyg-plugins.css nondeterministic build too
Piedone Apr 11, 2024
181eb6e
Revert "Debug change"
Piedone Apr 11, 2024
8f7548c
Adding ability to get the changed files from the artifact
Piedone Apr 11, 2024
c64b790
Temporarily not showing diffs
Piedone Apr 11, 2024
293ca47
Make the artifact upload actually run
Piedone Apr 11, 2024
c7345f3
Lower artifact retention period
Piedone Apr 11, 2024
a1faf3f
Fixing artifact path input
Piedone Apr 11, 2024
67a5f3d
Another attempt
Piedone Apr 11, 2024
67d30db
PS syntax fix
Piedone Apr 11, 2024
730a8ae
Fix attempt for env var writing
Piedone Apr 11, 2024
f9833f7
Reapply "Trying standardized line endings for websoor folders"
Piedone Apr 11, 2024
a5ffb71
Message hinting at the artifact
Piedone Apr 11, 2024
dc23038
Set the EOL only for JS and CSS files
Piedone Apr 11, 2024
8ddb17a
Changing built asset files to LF line endings
Piedone Apr 11, 2024
965b0a4
Enforcing LF EOL for all asset files
Piedone Apr 12, 2024
55c0875
Removing caching to rule out any differences in environments
Piedone Apr 12, 2024
c345146
Using media.js and TheAdmin.js from the CI
Piedone Apr 12, 2024
b1c327e
Disabling source maps, since these will always be different in CI
Piedone Apr 12, 2024
08354fb
generateSourceMaps default false again
Piedone Apr 12, 2024
3602945
Using the files from CI
Piedone Apr 12, 2024
8bf5b96
Re-add diff output
Piedone Apr 12, 2024
57089dd
Removing problematic files to recreate them in the next commit
Piedone Apr 12, 2024
d003bdd
Re-adding files
Piedone Apr 12, 2024
e09745b
Revert "Re-adding files"
Piedone Apr 12, 2024
27c6c99
Trying EOL fixed Sortable.js
Piedone Apr 12, 2024
cfa4549
Temporarily removing EOL settings
Piedone Apr 12, 2024
e6c3133
Revert "Trying EOL fixed Sortable.js"
Piedone Apr 12, 2024
cb72bf7
Adding LF files
Piedone Apr 12, 2024
d2f12d4
Revert "Temporarily removing EOL settings"
Piedone Apr 12, 2024
e32fb84
Missed an eol()
Piedone Apr 12, 2024
4e4d9ca
Filtering out "warning: in the working copy of ''path/to/file'', CRLF…
Piedone Apr 12, 2024
bc3009e
PS syntax
Piedone Apr 12, 2024
165f414
Revert filtering
Piedone Apr 12, 2024
83e3862
Filtering that actually works
Piedone Apr 12, 2024
1f635d1
Debug message
Piedone Apr 12, 2024
8b86921
No need for continue
Piedone Apr 12, 2024
6ad1bb5
Attempting a different match
Piedone Apr 12, 2024
10c19b8
Trying Contains()
Piedone Apr 12, 2024
85de141
Null check
Piedone Apr 12, 2024
ce1dc66
Debug output
Piedone Apr 12, 2024
cb3c483
Syntax
Piedone Apr 12, 2024
c62828c
Getting the diff via a temp file
Piedone Apr 12, 2024
f362fb1
Also redirecting stderr
Piedone Apr 12, 2024
88cce08
Using a variable again
Piedone Apr 12, 2024
f962cfa
Back to temp file
Piedone Apr 12, 2024
76c0e9d
Merge remote-tracking branch 'official/main' into issue/OCORE-158
Piedone Apr 16, 2024
4bc34bf
Fixing shortcodes.css
Piedone Apr 16, 2024
a818522
Code styling
Piedone Apr 16, 2024
fff395a
Hardcoding Media script build order too
Piedone Apr 16, 2024
04eee06
Hardcoding admin script build order too
Piedone Apr 16, 2024
9486eaf
EOL line break
Piedone Apr 16, 2024
d6551d0
Re-enabling concurrency config for assets_validation
Piedone Apr 16, 2024
a426492
Adding sorted glob support
Piedone Apr 17, 2024
fcf172b
Fixing trumbowyg-plugins.js
Piedone Apr 17, 2024
f4d0c1b
Trying to re-enable source maps
Piedone Apr 17, 2024
b57d7ec
Rename workflow
Piedone Apr 17, 2024
0fd3fd8
Temporarily disabling assets_validation concurrency prevention
Piedone Apr 17, 2024
6e51356
Revert "Trying to re-enable source maps"
Piedone Apr 17, 2024
56198f9
Testing source maps just with AdminDashboard
Piedone Apr 17, 2024
753e5ab
Revert "Testing source maps just with AdminDashboard"
Piedone Apr 17, 2024
afe50c2
Revert "Temporarily disabling assets_validation concurrency prevention"
Piedone Apr 17, 2024
10d155f
Removing outdated docs
Piedone Apr 17, 2024
03767fc
Removing debug console.logs
Piedone Apr 17, 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
6 changes: 5 additions & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
* text=auto

**/wwwroot/Scripts/* linguist-vendored
**/wwwroot/Scripts/* linguist-vendored

# Keep LF line endings in webroot assets files. Otherwise, building them under Windows would change the line endings to CLRF and cause changes without actually editing the source files.
**/wwwroot/**/*.js text eol=lf
**/wwwroot/**/*.css text eol=lf
100 changes: 89 additions & 11 deletions .github/workflows/assets_validation.yml
Original file line number Diff line number Diff line change
@@ -1,23 +1,29 @@
name: Frontend Assets Build Validation
on:
# Manual trigger.
workflow_dispatch:
name: Validating the Building of Public Assets

on:
push:
paths-ignore:
- '**/*.md'
- 'mkdocs.yml'
- 'src/docs/**/*'
branches: [ main ]
pull_request:
branches: [ main, release/** ]

concurrency:
group: ${{ github.head_ref || github.run_id }}-assets_validation
cancel-in-progress: true

jobs:
test-npm-build:
name: Test building assets
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/cache@v4
with:
path: ~/.npm
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-node-
- name: Rebuild packages
run: |
npm install
npm run rebuild
gulp rebuild
- name: Check if git has changes
shell: pwsh
run: |
Expand All @@ -28,9 +34,81 @@ jobs:
Write-Output 'Please make sure to build the assets properly before pushing, see https://docs.orchardcore.net/en/latest/docs/guides/gulp-pipeline/.'
Write-Output 'The following files changed:'
Write-Output $changes
Write-Output 'You can also download the attached artifact to see the changes.'
Write-Output ''
Write-Output '---------------------------------------'
Write-Output ''

$changeLines = $changes -split '`n'
$changedFiles = @()
$hasNonCrlfChange = $false

foreach ($line in $changeLines)
{
if ($line -match '^\s?(M|A|\?\?)\s+(.*)$')
{
$changeType = $matches[1]
$file = $matches[2]

Write-Output "Diff for: $file"

if ($changeType -eq 'M')
{
# File is modified; use git diff to get the diff of the modified file.

# The diff will be sent to stderr so we need to redirect it to stdout to capture it.
git diff -- $file 2>&1 >> tmp.txt
$diffOutput = Get-Content tmp.txt
Remove-Item tmp.txt

# Filtering out this pattern is necessary because certain CRLF line endings are not replaced by
# gulp-eol, so the output files can still have some CRLF.
if ($($diffOutput ?? '').Contains('CRLF will be replaced by LF the next time Git touches it'))
{
Write-Output "Warning: CRLF will be replaced by LF in $file. Fix this if you can, but certain CRLF line endings can't be replaced."
}
else
{
Write-Output $diffOutput
$hasNonCrlfChange = $true
}
}
elseif ($changeType -eq '??')
{
# File is (untracked); display the file contents.
Get-Content -Path $file
$hasNonCrlfChange = $true
}

$changedFiles += $file

Write-Output ''
Write-Output '---------------------------------------'
Write-Output ''
}
}

if (-not $hasNonCrlfChange)
{
Write-Output 'No non-CRLF changes found. Repository is clean.'
exit 0
}

# Convert the array of changed files to a single string with each file on a new line so actions/upload-artifact
# can consume it.
$changedFilesString = $changedFiles -join "`n"
"CHANGED_FILES<<ENDOFSTRING`n$($changedFilesString)`nENDOFSTRING" >> $Env:GITHUB_ENV

exit -1
}
else
{
Write-Host "No uncommitted changes found. Repository is clean."
}
- name: Upload changed files as artifact
uses: actions/upload-artifact@v4
if: failure()
with:
name: changed-files
path: ${{ env.CHANGED_FILES }}
retention-days: 30
24 changes: 16 additions & 8 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,19 @@ function getAssetGroups() {
}

function resolveAssetGroupPaths(assetGroup, assetManifestPath) {
assetGroup.manifestPath = assetManifestPath;
assetGroup.basePath = path.dirname(assetManifestPath);
assetGroup.inputPaths = assetGroup.inputs.map(function (inputPath) {
assetGroup.manifestPath = assetManifestPath.replace(/\\/g, '/');
assetGroup.basePath = path.dirname(assetGroup.manifestPath);
var inputPaths = assetGroup.inputs.map(function (inputPath) {
return path.resolve(path.join(assetGroup.basePath, inputPath)).replace(/\\/g, '/');
});

// For wildcard input paths also sortthem to ensure file concatenation is consistent.
if (inputPaths.some(path => path.includes('*'))) {
inputPaths = glob.sync(inputPaths, {}).sort();
}

assetGroup.inputPaths = inputPaths;

assetGroup.watchPaths = [];
if (!!assetGroup.watch) {
assetGroup.watchPaths = assetGroup.watch.map(function (watchPath) {
Expand Down Expand Up @@ -163,7 +171,7 @@ function buildCssPipeline(assetGroup, doConcat, doRebuild) {
if (ext !== ".scss" && ext !== ".less" && ext !== ".css")
throw "Input file '" + inputPath + "' is not of a valid type for output file '" + assetGroup.outputPath + "'.";
});
var generateSourceMaps = assetGroup.hasOwnProperty("generateSourceMaps") ? assetGroup.generateSourceMaps : true;
var generateSourceMaps = assetGroup.hasOwnProperty("generateSourceMaps") ? assetGroup.generateSourceMaps : false;
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
var generateRTL = assetGroup.hasOwnProperty("generateRTL") ? assetGroup.generateRTL : false;
var containsLessOrScss = assetGroup.inputPaths.some(function (inputPath) {
var ext = path.extname(inputPath).toLowerCase();
Expand Down Expand Up @@ -203,7 +211,7 @@ function buildCssPipeline(assetGroup, doConcat, doRebuild) {
.pipe(rename({
suffix: ".min"
}))
.pipe(eol())
.pipe(eol('\n'))
.pipe(gulp.dest(assetGroup.outputDir));
// Uncomment to copy assets to wwwroot
//.pipe(gulp.dest(assetGroup.webroot));
Expand All @@ -224,7 +232,7 @@ function buildCssPipeline(assetGroup, doConcat, doRebuild) {
.pipe(gulpif(doConcat, concat(assetGroup.outputFileName)))
.pipe(gulpif(generateRTL, postcss([rtl()])))
.pipe(gulpif(generateSourceMaps, sourcemaps.write()))
.pipe(eol())
.pipe(eol('\n'))
.pipe(gulp.dest(assetGroup.outputDir));
// Uncomment to copy assets to wwwroot
//.pipe(gulp.dest(assetGroup.webroot));
Expand All @@ -237,7 +245,7 @@ function buildJsPipeline(assetGroup, doConcat, doRebuild) {
if (ext !== ".ts" && ext !== ".js")
throw "Input file '" + inputPath + "' is not of a valid type for output file '" + assetGroup.outputPath + "'.";
});
var generateSourceMaps = assetGroup.hasOwnProperty("generateSourceMaps") ? assetGroup.generateSourceMaps : true;
var generateSourceMaps = assetGroup.hasOwnProperty("generateSourceMaps") ? assetGroup.generateSourceMaps : false;
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
// Source maps are useless if neither concatenating nor transforming.
if ((!doConcat || assetGroup.inputPaths.length < 2) && !assetGroup.inputPaths.some(function (inputPath) { return path.extname(inputPath).toLowerCase() === ".ts"; }))
generateSourceMaps = false;
Expand Down Expand Up @@ -292,7 +300,7 @@ function buildJsPipeline(assetGroup, doConcat, doRebuild) {
.pipe(rename({
suffix: ".min"
}))
.pipe(eol())
.pipe(eol('\n'))
.pipe(gulp.dest(assetGroup.outputDir))
// Uncomment to copy assets to wwwroot
//.pipe(gulp.dest(assetGroup.webroot));
Expand Down
Loading