Modernize manager template build workflow #15793

smg6511 · 2021-08-14T04:01:06Z

What does it do?

Replaces uglifyjs with terser to support the current javascript standard (ECMA2015/ES6) and support future stable standards
Bring other dev dependencies up to date
Add back in the copyright banner for compiled JS and CSS files (the current config uses an option that was removed from cssmin/clean-css many years ago)
Added new grunt task to just run the compile/minification routines for SASS/CSS
Removed imageoptim, as the only image used in the interface of any consequence (size-wise) is the login background
Made change to format of comments in the variables SASS file so they can be discarded during the build process
Removed reference to source map file in the popper.min.js file to remove the console error it was causing (due to the map file not existing)

Why is it needed?

Two reasons:

Virtually every tutorial and page of official documentation you will find on the web today prefers ES6 methods and support is robust among modern and semi-modern browsers. It's safe and prudent for MODX to adopt this standard, as its many great features allow more efficient coding and better scope control.
Dart SASS is the new and supported standard and the move to it will provide support for features that Node SASS will not (particularly moving into the future).

How to test

Run npm update in the _build/templates/default directory and run the grunt build, style, and compress tasks to verify.
Add a test method to the manager/assets/modext/util/utilities.js utilizing some ES6 syntax to verify it compiles correctly (running the compress or build task).
Verify in various target browsers that the changes do not adversely affect the manager's operation and styling looks the same as before the change.

Note that a warning will appear when compiling the SASS files, as fontawesome and normalize-scss use the to-be deprecated "/" for division. This will be replaced my the math.div([numerator], [denominator]) method and be required for Dart Sass 2.0+ (which is a ways off, as its current version is 1.37.5). This issue is on the radar for both repos.

Related issue(s)/PR(s)

None

JoshuaLuckers · 2021-08-26T13:17:44Z

I'm getting the following error when running npm build/grunt build:

Loading "dart-sass.js" tasks...ERROR
>> Error: Cannot find module 'sass'
>> Require stack:
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt-dart-sass/tasks/dart-sass.js
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/lib/grunt/task.js
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/lib/grunt.js
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/node_modules/grunt-cli/bin/grunt
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/bin/grunt
Loading "terser.js" tasks...ERROR
>> Error: Cannot find module 'terser'
>> Require stack:
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt-terser/tasks/terser.js
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/lib/grunt/task.js
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/lib/grunt.js
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/node_modules/grunt-cli/bin/grunt
>> - /Users/joshua/Projects/modx-3x/_build/templates/default/node_modules/grunt/bin/grunt
Warning: Task "dart-sass:dist" not found. Use --force to continue.

Aborted due to warnings.

smg6511 · 2021-08-26T14:36:24Z

Joshua - may be a silly question, but did you run npm update before trying to build? If not, you'll need to do that to install the new dev dependencies.

opengeek · 2021-08-26T19:45:42Z

I am unable to get past step 1 of the "How to test" instructions…

opengeek@drumshaman  ~/www/revo   3.x-mgr-tpl-build-updates  git rebase 3.x
Successfully rebased and updated refs/heads/3.x-mgr-tpl-build-updates.
 opengeek@drumshaman  ~/www/revo   3.x-mgr-tpl-build-updates  cd _build/templates/default
 opengeek@drumshaman  ~/www/revo/_build/templates/default   3.x-mgr-tpl-build-updates  npm update

> @fortawesome/fontawesome-free@5.15.4 postinstall /Users/opengeek/www/revo/_build/templates/default/node_modules/@fortawesome/fontawesome-free
> node attribution.js

Font Awesome Free 5.15.4 by @fontawesome - https://fontawesome.com
License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License)

npm WARN grunt-autoprefixer@3.0.4 requires a peer of grunt@>=0.4.2 but none is installed. You must install peer dependencies yourself.
npm WARN grunt-contrib-concat@1.0.1 requires a peer of grunt@>=0.4.0 but none is installed. You must install peer dependencies yourself.

+ @fortawesome/fontawesome-free@5.15.4
removed 366 packages, updated 1 package and audited 84 packages in 4.528s
found 6 vulnerabilities (1 moderate, 5 high)
  run `npm audit fix` to fix them, or `npm audit` for details

> node@14.17.4 preinstall /Users/opengeek/www/revo/_build/templates/default/node_modules/node
> node installArchSpecificPackage

+ node-darwin-x64@14.17.4
added 1 package in 2.727s
found 0 vulnerabilities

npm WARN grunt-autoprefixer@3.0.4 requires a peer of grunt@>=0.4.2 but none is installed. You must install peer dependencies yourself.
npm WARN grunt-contrib-concat@1.0.1 requires a peer of grunt@>=0.4.0 but none is installed. You must install peer dependencies yourself.

+ node@14.17.4
updated 1 package and audited 84 packages in 4.462s
found 6 vulnerabilities (1 moderate, 5 high)
  run `npm audit fix` to fix them, or `npm audit` for details
 opengeek@drumshaman  ~/www/revo/_build/templates/default   3.x-mgr-tpl-build-updates ●  git status
On branch 3.x-mgr-tpl-build-updates
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   package-lock.json
	modified:   package.json

no changes added to commit (use "git add" and/or "git commit -a")
 opengeek@drumshaman  ~/www/revo/_build/templates/default   3.x-mgr-tpl-build-updates ●  git diff
 ✘ opengeek@drumshaman  ~/www/revo/_build/templates/default   3.x-mgr-tpl-build-updates ●  grunt build
grunt-cli: The grunt command line interface (v1.3.2)

Fatal error: Unable to find local grunt.

If you're seeing this message, grunt hasn't been installed locally to
your project. For more information about installing and configuring grunt,
please see the Getting Started guide:

https://gruntjs.com/getting-started

opengeek

This is not working as described.

smg6511 · 2021-08-26T22:19:27Z

Sorry about that, guys. Let me update the package.json to get the correct grunt locally. I had no problems, but must have been running against my globally installed grunt module. Be back...

smg6511 · 2021-08-27T02:22:41Z

After researching a bit I suspect you may be running a version of npm between v3 and v6, which do not automatically resolve peer dependencies. If so, upgrading to the current stable version (7.x) will hopefully clear up the issue (npm install -g npm@latest).

JoshuaLuckers · 2021-08-27T08:17:07Z

After researching a bit I suspect you may be running a version of npm between v3 and v6, which do not automatically resolve peer dependencies. If so, upgrading to the current stable version (7.x) will hopefully clear up the issue (npm install -g npm@latest).

That might be the case, I'll try it again soon!

JoshuaLuckers · 2021-08-27T12:31:36Z

Upgrading npm to 7.x solved my issue. I was running 6.x.

I got it working

_build/templates/default/package.json

matdave

Seems to work for me.

Updates to dev packages and the grunt config primarily to support ES6 javascript and replace the deprecated node sass implementation with dart sass.

Provides a more streamlined config for autoprefixing and minification via postCSS module and its plugins and removes issues with grunt-autoprefixer dependency vulnerabilities

mishanthrop · 2021-09-27T13:20:50Z

_build/templates/default/package.json

+        "grunt-terser": "^2.0.0",
+        "node": "^14.15.0",
+        "normalize-scss": "^7.0.1",
+        "postcss": "^8.3.6"


Why you use postcss and sass both?

postcss is being used as a base for cssnano and autoprefixer. I'm not using the postcss scss plugin to parse scss because it's not entirely standard sass.

mishanthrop · 2021-09-27T13:23:17Z

_build/templates/default/package.json

+        "grunt-dart-sass": "^2.0.1",
+        "grunt-notify": "^0.4.5",
+        "grunt-terser": "^2.0.0",
+        "node": "^14.15.0",


windows 7 is not supported by node.js 14 :( Windows 7 still be more popular than 8, at least in Russia.
Is this dependency is necessary?

Windows 7, as you probably know, has been unsupported by Microsoft for more that a year and a half now. That said, I've seen that people on 7 have been able to get node 14+ running. See nodejs/node#33000 (comment)

Thanks for link, I'll try it.
Anyway, I would ask again: for what purpose is this dependency?

It's the base software needed to use npm and the modules it manages. Are you intending on doing some development on the manager interface? Note that this same dependency has existed in 2.x for quite a while now.

It's very strange to have the npm installed and not have the nodejs installed, isn't it?

@mishantrop yes it is. I assume you're running Windows 7?

Yes, including. Also I use Fedora and Mac OS. Dependings on tasks.

Also there are better ways to limit the version of the application (anyway, I dont see any reasons for it in MODX):

package.json

{ "name": "modx", "version": "3.0.0", "engines": { "node": ">=12.0.0" } }

.npmrc

engine-strict=true

I have not personally tested it as it was never necessary.

Source: https://stackoverflow.com/a/68653111

smg6511 requested review from Mark-H and opengeek as code owners August 14, 2021 04:01

cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Aug 14, 2021

Ibochkarev added the pr/review-needed Pull request requires review and testing. label Aug 25, 2021

opengeek requested a review from matdave August 26, 2021 19:19

opengeek previously requested changes Aug 26, 2021

View reviewed changes

opengeek self-requested a review August 27, 2021 19:56

JoshuaLuckers reviewed Aug 28, 2021

View reviewed changes

_build/templates/default/package.json Show resolved Hide resolved

JoshuaLuckers approved these changes Aug 28, 2021

View reviewed changes

smg6511 mentioned this pull request Aug 30, 2021

Refactor template variable creation #15773

Merged

matdave approved these changes Sep 3, 2021

View reviewed changes

Ibochkarev added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Sep 3, 2021

Ibochkarev added this to the v3.0.0-alpha3 milestone Sep 3, 2021

Jim Graham and others added 3 commits September 16, 2021 13:42

Modernization of Template Build Workflow

dfb8e11

Updates to dev packages and the grunt config primarily to support ES6 javascript and replace the deprecated node sass implementation with dart sass.

Change CSS Post-processing Method

35a3676

Provides a more streamlined config for autoprefixing and minification via postCSS module and its plugins and removes issues with grunt-autoprefixer dependency vulnerabilities

Run npm update and grunt build

fbcd42e

opengeek force-pushed the 3.x-mgr-tpl-build-updates branch from 0612ce1 to fbcd42e Compare September 16, 2021 19:45

opengeek changed the title ~~Modernization of Template Build Workflow~~ Modernize Template Build Workflow Sep 16, 2021

opengeek changed the title ~~Modernize Template Build Workflow~~ Modernize manager template build workflow Sep 16, 2021

opengeek merged commit ccaf7a5 into modxcms:3.x Sep 16, 2021

rthrash removed the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Sep 16, 2021

mishanthrop reviewed Sep 27, 2021

View reviewed changes

smg6511 mentioned this pull request Oct 2, 2021

Setup a11y: using the Tab key to select the language #15823

Merged

theboxer mentioned this pull request Jan 6, 2022

[3.x] Form Customization: Grid editing erases set data #15954

Closed

smg6511 deleted the 3.x-mgr-tpl-build-updates branch July 19, 2022 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize manager template build workflow #15793

Modernize manager template build workflow #15793

smg6511 commented Aug 14, 2021 •

edited

Loading

JoshuaLuckers commented Aug 26, 2021

smg6511 commented Aug 26, 2021

opengeek commented Aug 26, 2021

opengeek left a comment

smg6511 commented Aug 26, 2021

smg6511 commented Aug 27, 2021 •

edited

Loading

JoshuaLuckers commented Aug 27, 2021

JoshuaLuckers commented Aug 27, 2021

matdave left a comment

mishanthrop Sep 27, 2021

smg6511 Sep 27, 2021 •

edited

Loading

mishanthrop Sep 27, 2021

smg6511 Sep 27, 2021 •

edited

Loading

mishanthrop Sep 27, 2021

smg6511 Sep 27, 2021

mishanthrop Sep 27, 2021

JoshuaLuckers Sep 28, 2021

mishanthrop Oct 2, 2021 •

edited

Loading

mishanthrop Oct 2, 2021 •

edited

Loading

Modernize manager template build workflow #15793

Modernize manager template build workflow #15793

Conversation

smg6511 commented Aug 14, 2021 • edited Loading

What does it do?

Why is it needed?

How to test

Related issue(s)/PR(s)

JoshuaLuckers commented Aug 26, 2021

smg6511 commented Aug 26, 2021

opengeek commented Aug 26, 2021

opengeek left a comment

Choose a reason for hiding this comment

smg6511 commented Aug 26, 2021

smg6511 commented Aug 27, 2021 • edited Loading

JoshuaLuckers commented Aug 27, 2021

JoshuaLuckers commented Aug 27, 2021

matdave left a comment

Choose a reason for hiding this comment

mishanthrop Sep 27, 2021

Choose a reason for hiding this comment

smg6511 Sep 27, 2021 • edited Loading

Choose a reason for hiding this comment

mishanthrop Sep 27, 2021

Choose a reason for hiding this comment

smg6511 Sep 27, 2021 • edited Loading

Choose a reason for hiding this comment

mishanthrop Sep 27, 2021

Choose a reason for hiding this comment

smg6511 Sep 27, 2021

Choose a reason for hiding this comment

mishanthrop Sep 27, 2021

Choose a reason for hiding this comment

JoshuaLuckers Sep 28, 2021

Choose a reason for hiding this comment

mishanthrop Oct 2, 2021 • edited Loading

Choose a reason for hiding this comment

mishanthrop Oct 2, 2021 • edited Loading

Choose a reason for hiding this comment

smg6511 commented Aug 14, 2021 •

edited

Loading

smg6511 commented Aug 27, 2021 •

edited

Loading

smg6511 Sep 27, 2021 •

edited

Loading

smg6511 Sep 27, 2021 •

edited

Loading

mishanthrop Oct 2, 2021 •

edited

Loading

mishanthrop Oct 2, 2021 •

edited

Loading