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

[9.x] Webpack 5 / fix issues #2607

Merged
merged 40 commits into from Nov 27, 2021
Merged

[9.x] Webpack 5 / fix issues #2607

merged 40 commits into from Nov 27, 2021

Conversation

strarsis
Copy link
Contributor

@strarsis strarsis commented Jan 9, 2021

@retlehs: Similar to the PR for updating Sage 9.x to webpack 4, this PR updates to webpack 5 instead. However, this will break node v8 downwards compatibility, as webpack 5 and other dependencies dropped node 8.x support, therefore this separate PR is created.

Sage 9.x branch forum discussion: https://discourse.roots.io/t/sage-9-1-please-test/20067?u=strarsis

@strarsis strarsis changed the base branch from master to 9.x January 9, 2021 22:42
@strarsis
Copy link
Contributor Author

strarsis commented Jan 13, 2021

As Sage 9 uses the sage-installer to add framework-specific adjustments, sage-installer also has to be updated using this complimenting PR:
roots/sage-installer#43

You can test it using composer create-project:
https://discourse.roots.io/t/sage-9-2-please-test/20067

Replace deprecated [hash:8] placeholder with [fullhash:8].
Use deprecated `[hash]` instead of `[fullhash]` until supported with contrib copy plugin.
Remove friendly error messages plugin (unmaintained).
… (and contrib copy plugin).

Add noErrorOnMissing option to copy plugin to prevent errors for empty asset subfolders.
@strarsis
Copy link
Contributor Author

@retlehs: Would it be possible to merge this into Sage 9 branch?

@Log1x Log1x added the 9.x Sage 9 related label Feb 16, 2021
CHANGELOG.md Outdated Show resolved Hide resolved
jeffbyrnes added a commit to abundanthomesma/abundanthousingma.org that referenced this pull request Mar 7, 2021
@strarsis
Copy link
Contributor Author

strarsis commented Jun 2, 2021

Developer note: For using the sage-installer framework-specific modifications on top of a checked out repository (not using composer create-project):

composer install
composer run-script post-create-project-cmd

jeffbyrnes added a commit to abundanthomesma/abundanthousingma.org that referenced this pull request Jun 3, 2021
Copy link

@j-funk j-funk left a comment

Choose a reason for hiding this comment

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

This was the easiest migration to webpack 5 I've done - kudos and thank you, so many dependencies don't work with webpack 3 anymore.

resources/assets/build/config.js Show resolved Hide resolved
package.json Outdated
"rimraf": "^3.0.2",
"sass-loader": "~6.0",
"style-loader": "^0.23.1",
"sass": "^1.32.0",
Copy link

Choose a reason for hiding this comment

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

you might want to pin this, for now, I ran into this issue and removing carat fixed it -- twbs/bootstrap#34051

Copy link
Contributor Author

@strarsis strarsis Jul 11, 2021

Choose a reason for hiding this comment

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

@j-funk: Recently I re-added the friendly errors plugin. This should basically fix the deprecation warnings issue as those are automatically cleared at the end, with just the warnings/errors being logged.

See the corresponding Sage 9.x update branch discussion in the forum:
https://discourse.roots.io/t/sage-9-1-please-test/20067/72?u=strarsis

'nav-walker',
'nice-search',
'relative-urls'
]);

Choose a reason for hiding this comment

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

It appears that the soil-jquery-cdn feature has been removed, was that intentional?

Copy link
Contributor Author

@strarsis strarsis Aug 28, 2021

Choose a reason for hiding this comment

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

Hm, it should enable the same features as Sage 10 does.
Apparently the soil-jquery-cdn feature was removed from Soil, see this PR: roots/soil#228

package.json Outdated Show resolved Hide resolved
@strarsis
Copy link
Contributor Author

strarsis commented Sep 15, 2021

Some dependencies released new major versions in the meantime. With these newer dependencies the build setup now natively handles url. If you want to try the latest dependencies out, you can check this branch: https://github.com/strarsis/sage/tree/webpack5-url-update
This also seems to restore the previous behaviour where the url paths are resolved relative to the file using it, instead to resources/ folder.

@mrkaluzny
Copy link

@strarsis I'm working with your webpack-5 branch update, the only issue I see is the problem with rendering background-images from urls. While developing the correct path to the asset won't resolve. It's working correctly with builds using ./images/filename. Any idea what could cause the issue?

Also I had some errors thrown below node v16 (Image optimization issues)

@strarsis
Copy link
Contributor Author

@mrkaluzny: The latest update of the webpack5 restored the old behavior of resolving the paths:
They are again relative to the stylesheet file, as in current Sage 9 master.

Yes, the image optimization issues are probably due to bindings to image manipulation libraries (like sharp)
that have a tendency to go wrong. Using the most recent dependencies and also node runtime often fixes all these issues.
Is node support for versions below v16 important for you?
I always use a node version manager (nvm is great) with a .nvmrc file that locks the node version.
This helps to ensure that the same node version (or known release) for the project is used across systems and team members.

@mrkaluzny
Copy link

@strarsis node version might be a problem with production builds. Right now I’m trying to fix issues with our setup (I’ve set up our starter based on Sage 9.0.1 with support for Tailwind v2).

But we had the same issue (no way to get assets in css). It’s better with your setup (production works as expected) but when running watch on files the images and fonts are not loaded. The assets are available in ‘dist’ folder but when requested are loading as blank. Have you tried anything to fix this? Or have time for a quick chat?

We’re doing quite a bit of work based on Sage and it’s starting to become a large headache

@strarsis
Copy link
Contributor Author

strarsis commented Oct 27, 2021

@mrkaluzny: Please try the webpack5-url-update branch, which will be merged into the webpack5 branch at some point (after some testing).
It uses the most recent loaders - this can be already enough to fix the path resolving issue during watching.

Edit: Does it fix the issue?

@jeffbyrnes
Copy link

jeffbyrnes commented Oct 29, 2021

Wondering if there's any way this could get rolled in to the upcoming v10.0.0 release? I see that 10.0.0-beta.1 is already out there.

Ignore me, I was totally spacing on how this is squarely aimed at v9.x.

@retlehs retlehs merged commit d2604f7 into roots:9.x Nov 27, 2021
@strarsis
Copy link
Contributor Author

strarsis commented Jan 3, 2022

@retlehs: That's great! One improvement (but not very important) could be made:
I added the corresponding update branch of the sage-installer composer package as dependency to make creating projects from the update branch possible: https://github.com/roots/sage/blob/9.x/composer.json#L35,L43
This was necessary because for Sage 9.x themes the sage-installer has to be used to apply framework-specific adjustments on top of it during the project creation (composer script).

As this has been merged now and sage-installer also was updated,
it would now make sense to publish a new release of sage-installer (current release (tailwind stuff, doesn't include the fixes for sage-installer) is v1.6.4, a new release could be v1.7.0 or v2.0.0) and revert the sage theme dependency back to the official sage-installer package: d2604f7#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
9.x Sage 9 related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants