Discussion: Version 3 release

bholloway commented 5 years ago

Finally rework has been updated to postcss!

An alpha is available as resolve-url-loader@next. This dist-tag will be updated as we near release.

Refer to the readme for migration instructions.

Breaking Changes

Multiple options changed or deprecated.
Removed file search "magic" in favour of join option.
Errors always fail and are no longer swallowed.
Processing absolute asset paths requires root option to be set.

Feedback required

Moving forward on this release is contingent to a few questions..

[x] is your existing build unchanged when using engine: rework ?
[x] is your build working when using engine: postcss (i.e. the new default) ?
[ ] do you need the file-search "magic" that was removed; is your use-case broken without it ?

And it would be nice to know...

[x] does this fix any existing issues ?

Maturity

For an alpha this seems (to me) pretty robust.

The only API that is in question is the join function. This is advanced feature and is unlikely to be needed by most users. If you intend to use a custom join (in place of the file search "magic") then please let me know.

I've not tested any complex (real-world) builds. If you have a use-case you can encode into an example project then I'm definitely interested.

n-rodriguez commented 5 years ago

Hi there! I'm testing it on my app and it's already in production :)

Here's my setup :

webpack 4.17.1

const { environment } = require('@rails/webpacker')

// Load Webpack config
const bootstrap          = require('./loaders/bootstrap')
const datatables         = require('./loaders/datatables')
const erb                = require('./loaders/erb')
const jquery             = require('./loaders/jquery')
const modernizr_loader   = require('./loaders/modernizr_loader')
const modernizr_resolver = require('./loaders/modernizr_resolver')
const yadcf              = require('./loaders/yadcf')
const webpack            = require('webpack')

// Load resolve-url-loader
environment.loaders.get('sass').use.splice(-1, 0, {
  loader: 'resolve-url-loader'
})

// Replace existing coffee loader with CS2 version
const babel_loader = environment.loaders.get('babel')
environment.loaders.insert('coffee', {
  test: /\.coffee(\.erb)?$/,
  use:  babel_loader.use.concat(['coffee-loader'])
})

// Add Webpack loaders
environment.loaders.append('bootstrap', bootstrap)
environment.loaders.append('datatables', datatables)
environment.loaders.append('erb', erb)
environment.loaders.append('jquery', jquery)
environment.loaders.append('modernizr_loader', modernizr_loader)
environment.loaders.append('yadcf', yadcf)

// Merge Modernizr config
environment.config.merge(modernizr_resolver)

// Export configured environment
module.exports = environment

It solves an issue with JQueryUI where images were not exported.

Thank you!

rjgotten commented 5 years ago

As requested in #98, some feedback on the two engines:

engine:rework seems to work without regression; but continues the trend of having path corruption in source map sources when building on Windows systems.

engine:postcss lessens the above path corruption considerably. (Only repeats the rooted disk path once, rather than n times.) However, the content of mapped sources is lost.

n-rodriguez commented 5 years ago

However, the content of mapped sources is lost.

The same here. See https://github.com/bholloway/resolve-url-loader/issues/98#issuecomment-417072239.

bholloway commented 5 years ago

So as discussed in #98 there is a fix for output source-map missing sourcesContent.

This is published as version 3.0.0-alpha.2 with updated dist-tag next.

n-rodriguez commented 5 years ago

This is published as version 3.0.0-alpha.2 with updated dist-tag next.

Awesome! It's working well :)

rjgotten commented 5 years ago

Working well sofar for the project I'm working on as well. Haven't heard of further issues than those resolved with alpha.2

bholloway commented 5 years ago

@rjgotten from my side I am checking correctness of a billion source-maps from the e2e tests. Following that I am happy to make a beta release.

Once at beta I am unsure. This is a big rewrite so I am wondering how cautious to be.

Either

First publish a patch of version 2 to npm that publicises the @next version in capitals.
Just push the button on release and hope for the best.

Happy to take thoughts. If there is any regressions with release then I might be having to make frantic fixes and imposing on those here to retest.

bholloway commented 5 years ago

On the subject of outgoing source-maps...

engine:postcss 👍

The mappings look good for the 10 cases in the e2e tests. I reviewed both development and production output.

The e2e tests are relatively simple cases but it gives some confidence, and really this is postcss concern anyhow.

engine:rework

The mappings look okay in development. 👍 They look okay in production for webpack < 4 👍 However they seem to be systematically flawed in production for webpack = 4 💩

bholloway commented 5 years ago

This is published as version 3.0.0-beta.1 with updated dist-tag next.

bholloway commented 5 years ago

`3.0.0-beta.1`

Please refer to the beta on npm. Install using dist-tag resolve-url-loader@next.

As per the readme, version 3 brings postcss parser.

Please try the beta and vote :thumbsup: or :thumbsdown: for release based on whether it works for your use case.

If you vote :thumbsdown: please post detail any problems below.

bholloway commented 5 years ago

Only 3 thumbs in a week. But I am inclined to release (today?) anyhow.

Either way I think it is time to be on master per PR #101 .

bholloway commented 5 years ago

🎉 Published as resolve-url-loader@3.0.0 🎉

Thanks for all your help!

I shall close the issue but please feel free to continue commenting.

bholloway / resolve-url-loader

Discussion: Version 3 release #97

`3.0.0-beta.1`