search

bertramdev / asset-pipeline

The core implementation of the asset pipeline for the jvm
193 stars 91 forks source link

Grails 4.1.0.M1 "Accessing config through dot notation is deprecated" #261

Closed davidkron closed 4 years ago

davidkron commented 4 years ago

Im doing some Grails 4.1.0.M1 testing with some of our applications and I get a lot of warnings:

org.grails.config.NavigableMap: Accessing config through dot notation is deprecated, and it will be removed in a future release. Use 'config.getProperty(key, targetClass)' instead.

Some of these warnings are caused by the the asset-pipeline plugin on every request.

davydotcom commented 4 years ago

fixed in 3.2.2

davidkron commented 4 years ago

I'm not really sure this is really fixed, we still get plenty of warnings in our codebase.

I placed debugging breakpoints in these locations: https://github.com/grails/grails-core/blob/bdd4bc86b7fde4da4aaec99bbb1e82eb9db7dee7/grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy#L298 https://github.com/grails/grails-core/blob/bdd4bc86b7fde4da4aaec99bbb1e82eb9db7dee7/grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy#L444

Using these debug breakpoints I tracked the following locations, which still use the deprecated dot notation: https://github.com/bertramdev/asset-pipeline/blob/e47a2239013e8c9f9eac6603033614e6f7b29497/asset-pipeline-grails/src/main/groovy/asset/pipeline/grails/AssetProcessorService.groovy#L51 https://github.com/bertramdev/asset-pipeline/blob/e47a2239013e8c9f9eac6603033614e6f7b29497/asset-pipeline-grails/src/main/groovy/asset/pipeline/grails/AssetProcessorService.groovy#L54 https://github.com/bertramdev/asset-pipeline/blob/e47a2239013e8c9f9eac6603033614e6f7b29497/asset-pipeline-grails/src/main/groovy/asset/pipeline/grails/AssetProcessorService.groovy#L64 https://github.com/bertramdev/asset-pipeline/blob/e47a2239013e8c9f9eac6603033614e6f7b29497/asset-pipeline-grails/src/main/groovy/asset/pipeline/grails/AssetProcessorService.groovy#L103 https://github.com/bertramdev/asset-pipeline/blob/e47a2239013e8c9f9eac6603033614e6f7b29497/asset-pipeline-grails/src/main/groovy/asset/pipeline/grails/AssetProcessorService.groovy#L112

davydotcom commented 4 years ago

Ah, good find, I missed the embedded method default uses. My bad. ill publish an update

davidkron commented 4 years ago

I'm sorry that I have to touch this issue again, but I think the fix didn't have the desired behavior. I tested this with the newest 3.2.4 version.

The problem seems to be something like this:

def config = grailsApplication.config.getProperty('grails.assets',Map,[:])
println config.someProperty

Since the node grails.assets is a NavigableMap and the expected type is a Map as well, there will be no type conversion. And the call later to get a property triggers the warning again.

These are likely the places that still trigger the warning: https://github.com/bertramdev/asset-pipeline/blob/1192aa19d9ed792e18ab5ed534bdfa5e570fad1b/asset-pipeline-grails/src/main/groovy/asset/pipeline/grails/AssetProcessorService.groovy#L104 https://github.com/bertramdev/asset-pipeline/blob/1192aa19d9ed792e18ab5ed534bdfa5e570fad1b/asset-pipeline-grails/src/main/groovy/asset/pipeline/AssetPipelineGrailsPlugin.groovy#L123 https://github.com/bertramdev/asset-pipeline/blob/1192aa19d9ed792e18ab5ed534bdfa5e570fad1b/asset-pipeline-grails/grails-app/taglib/asset/pipeline/grails/AssetsTagLib.groovy#L84

Also I think the conf parameters in the methods getAssetPath and getResolvedAssetPath could be removed as conf is not referenced inside the method.