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

Update Template Build Process #16540

Merged
merged 3 commits into from
Mar 21, 2024

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Mar 21, 2024

What does it do?

Replaces most legacy dependencies with current versions—with the exception of bourbon, neat, and fontawesome—and drops others that are no longer relevant (such as imageoptim).

Why is it needed?

Bring 2.x more in line with 3.x, mainly allow use of modern js features.

How to test

  1. Run the rebuild processes, including npm update within the _build/templates/default directory.
  2. Run grunt build.
  3. Clear your manager and browser cache, then browse around the manager with your console open to verify all works as expected and no errors are being reported.

Note that grunt build will spit out some warnings, as the versions of bourbon and neat we need to stick with here (for now at least) are ancient and contain some long-deprecated code. I attempted to bring these dependencies up to date (including fontawesome) but there are many breaking changes that make it difficult to unwind and get everything working. Might try that later if there's enough "life" left in the 2.x line and it's deemed beneficial to do so.

Related issue(s)/PR(s)

Resolves issues with building after including the following PRs: #16493 and #16467.

Replaces most legacy dependencies with current versions; with the exception of bourbon, neat, and fontawesome.
@opengeek opengeek changed the title [2.x] Update Template Build Process Update Template Build Process Mar 21, 2024
@opengeek
Copy link
Member

Shouldn't this include the updated package-lock.json file? If I run npm update, changes are made to the package-lock.json, which then needs to be committed again.

@smg6511
Copy link
Collaborator Author

smg6511 commented Mar 21, 2024

Shouldn't this include the updated package-lock.json file?

I may be mistakenly thinking I shouldn't include it, which is why I didn't. I'll add a commit with it in just a bit...

@opengeek
Copy link
Member

Shouldn't this include the updated package-lock.json file?

I may be mistakenly thinking I shouldn't include it, which is why I didn't. I'll add a commit with it in just a bit...

Well, I am asking. In the PHP/composer world, you want the composer.lock file to set the dependencies to avoid differences when multiple people install the dependencies. I know that we do not include the composer.lock file in the revo project, but that is a special case since revo is essentially a framework. I would think we would want that in the case of the assets build, however. And if we don't, should it be removed from the repo?

@smg6511
Copy link
Collaborator Author

smg6511 commented Mar 21, 2024

I think you're right to want it in there after quick investigation.

@opengeek opengeek merged commit f7974d1 into modxcms:2.x Mar 21, 2024
5 checks passed
@smg6511 smg6511 deleted the 2.x-template-build-upgrade-js branch March 30, 2024 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants