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

Revert breaking change from V3 #42

Closed
wants to merge 1 commit into from

Conversation

homestar9
Copy link
Contributor

Prevent css-loader from trying to modify URLs. This was a breaking change from V3 that is not needed.

Prevent css-loader from trying to modify URLs.  This was a breaking change from V3 that is not needed.
@homestar9
Copy link
Contributor Author

Note: there is still a breaking issue in V4 that I have yet to figure out how to solve. This PR addresses the error that is generated in Webpack during a build.

The remaining issue is that URLs are renamed relative to the original path in SCSS files, when they should be untouched as they were in Elixir V3.

For example, lets say you have the following SCSS:

.brand{
  background-image: url( '/myColdboxModule/includes/img/logo.png' );
}

Elixir V4 will rename the URL in the resulting CSS file thus breaking the link.

.brand{
  background-image: url( '../../myColdboxModule/includes/img/logo.png' );
}

I do not know how to fix this issue and probably need some outside help to solve it.

@homestar9
Copy link
Contributor Author

@jclausen
Copy link
Collaborator

I can't merge this, as is, as it will break other implementations which are using images from a source directory and copying them over to final includes/images directory. The loaders for the current versions of Webpack are much more finicky about being able to locate the asset. They consider this a "feature" rather than a bug: https://github.com/webpack-contrib/sass-loader#problems-with-url

If I'm understanding correctly, you don't want those files referenced to be sourced in during the pack and consolidated to an
assets directory, but for webpack to ignore the bad link. If this is the case, then using elixir.config.mergeConfig({... may be your best solution so that you can customize the behavior you want. Then you can just update your webpack.config.js to use the url : false by merging a new config for handling .scss files.

This config addition, from the SO post linked above though might allow it to skip out if it can't locate the file.

{
  loader: 'resolve-url-loader',
  options: {
    attempts: 1,
    sourceMap: true
  }
}

I will test this with some root-relative links in existing apps to see if I can get it to ignore the bad paths. My main concern is that it won't re-attempt to find other assets.

@jclausen
Copy link
Collaborator

I've created Issue #44 to address this for future releases. My recommendation, for now, would be to update the links to the files to be resolvable from the root of the project, when compiled. As long as absolute URLs are resolvable, they are not compiled in to the includes/images directory.

.brand{
  background-image: url( '/modules_app/myCustomColdboxModule/includes/img/logo.png' );
}

@jclausen jclausen closed this Jan 26, 2023
@homestar9
Copy link
Contributor Author

@jclausen Thank you for the feedback and for creating issue 44.

My recommendation, for now, would be to update the links to the files to be resolvable from the root of the project, when compiled

Unfortunately, when building Coldbox modules, making links to assets resolvable from the root is not always easy. When developing, the module might load from a /test-harness/ folder and, in production, it would load via the modules/myModuleName folder. There's also no way to know how deep down in the dependency hierarchy a module's assets might exist from the app root (e.g. /modules/myModule1/modules/myModule2)

One way I have solved this problem in the past is by using web server folder aliases (aka rewrite/redirect rules) like this one in a module's test-harness folder:

// server.json in test-harness
{
    "web":{
        "http":{
            "port":63789
        },
        "aliases":{
            "/moduleroot/myModule/":"../", <-- alias to the module's real path
        }
    }
}

I make SCSS URLs based on the root, but webpack has no way of knowing that web server redirects/aliases handle the actual mapping. Does that make sense?

@jclausen
Copy link
Collaborator

jclausen commented Jan 26, 2023

It makes total sense. I deal with this issue with both the relax and the stachebox module. The way I handle it there is to use aliases in the test harness, when developing the module. https://github.com/coldbox-modules/stachebox/blob/development/test-harness/server-adobe%402018.json#L14

I can just use the standard elixirPath method in the layout, though, because it takes in to account whether the layout is being delivered from a module or not: https://github.com/coldbox-modules/stachebox/blob/development/layouts/Main.cfm#L16

When the module is deployed, though, the links work perfectly.

@homestar9
Copy link
Contributor Author

homestar9 commented Jan 26, 2023

I'm glad to know I'm not the only one! Haha.

In stachebox how would you handle linking to the logo file in your app.css?

.brand{
  background-image: url( '/stachebox/includes/img/stachebox-logo.png' );
}

The above link would cause an error when running Webpack, similar to what I experienced, wouldn't it? You would have to manually override some of the Webpack configuration so that URL's remain untouched in your CSS/SCSS

@jclausen
Copy link
Collaborator

The way I would do is is to use a relative link, since the sass-loader is going to move those files so the relative links just work, anyway. So if my file is in resources/assets/scss and my image is in resources/assets/img it would be:

.brand{
  background-image: url( '../img/stachebox-logo.png' );
}

You can also set it to inline the base64 of the image in to the generated CSS file and then it doesn't even need to copy it over. So, for example if you want any file less than 10,000kb to be inlined in to the CSS you can set this in your webpack.config.js file:

elixir.base64SourceSize = 10000000;

For small images like logos and other things, this is super handy.

@homestar9
Copy link
Contributor Author

Thank you! I will experiment with making the links relative.

@homestar9
Copy link
Contributor Author

@jclausen, sorry for the continued posts on this issue, but I have a related question regarding images. I'm coming from the Gulp world, so I am used to processing scss and images in separate tasks.

Elixir + Webpack appears to process scss + images together via sass-loader, as far as I can tell. When sass-loader comes across a reference to an image, it copies it from the /resources/assets/img/ folder to /includes/images/

For example:

.brand{
  background-image: url( '../img/logo.png' ); <-- sass-loader copies the image file and changes this to ../images/logo.png
}

I have two questions related to this process:

  1. Is there a way to tell Elixir to use /includes/img instead of /includes/images? Or is that the official convention?
  2. What if I want to do some type of 3rd party image minification/optimization, similar to gulp-imagemin?

I know I can write extensions for Elixir, but if sass-loader is taking over control and copying over every image it discovers, wouldn't this negate any image processing I set up in the future?

@homestar9
Copy link
Contributor Author

homestar9 commented Jan 28, 2023

I discovered another thing when switching to relative paths in my scss files.
Webpack converts relative paths like this:
url( "../../img/logo.png" );
to this:
url(/includes/images/logo.png);
Note the path is no longer relative to the scss file and instead relative to what it thinks is the root. However, if the asset is needed in a submodule, the link will break in the Webpack processed version.

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