Fix browser bundling with webpack 5

[barak007]

I think that the browser field is missing url: false.
I'm not completely sure that this is the right fix and also the npm package url does not implement the APIs used by postcss.

steps to reproduce

mkdir postcss-browser 
cd postcss-browser 
mkdir src
echo 'import "postcss";' > src/index.js
npm init -y
npm install webpack webpack-cli postcss
npx webpack

This will emit error:

ERROR in ./node_modules/postcss/lib/input.js 3:39-53
Module not found: Error: Can't resolve 'url' in 'C:\projects\postcss-web-bundle\postcss-browser\node_modules\postcss\lib'

[ai]

Can I ask you to test this branch in webpack 4 and webpack 5?

[barak007]

I didn't. I will do later today. But I guess that any way the webpack 4 built-in polyfill is also wrong since it's based on the same url polyfill.

[ai]

"url": false will force require('url') to return empty object {}. It could break source map generation.

[barak007]

True (it will throw an error for the missing function). No one today that uses postcss in the browser have those functions available for them (unless there is a polyfill I don't know about). I truly consider to create a proper polyfill for them but up until now there isn't any.

[ai]

It will be breaking changes

[ai]

Because we will break webpack 4 build. We need a solution to support both bundlers without config changes (or with some migration help).

[barak007]

At the current state of things:

• webpack@4 users get a partial polyfill that does not include the used functions. I suspect the only reason it didn't throw at runtime is because that branch in the flow was never called by a consumer using it inside a browser.
• webpack@5 users either get an error of missing "url" or implicitly get the same partial polyfill if it's included in node_modules by another dependency.

I propose one of two possible solutions:

1. Include a small (posix only) polyfill for the two used functions when running in the browser. webpack@4 and webpack@5 users will both be able to bundle postcss, with the code getting the correct polyfilled functions at runtime.
2. Map url to false and protect the flow that uses those functions.

Assuming the above is correct, either approach will probably not be a breaking change, but a bugfix.

[ai]

Include a small (posix only) polyfill for the two used functions

I like the idea

[barak007]

After deep investigation I notice few things:

Source map generation is disabled when bundling for browser since path is mapped to false (this is why it didn't break)

postcss/lib/map-generator.js

Line 280 in 8682b1e

if (pathAvailable && this.isMap()) {

And the other two locations that uses url are not really that important

1. When the from is file protocol
   

   postcss/lib/input.js

   Line 164 in 8682b1e

   if (fromUrl.protocol === 'file:') {

2. When throwing an error and annotating the results with the url
   

   postcss/lib/input.js

   Line 133 in 8682b1e

   result.input.url = pathToFileURL(this.file).toString()

I did managed to create the polyfill but I'm now thinking maybe disable these features will be good also.

If not maybe we should consider to also to implement the path behavior required for the entire code to work. (path.resolve is already in-lined inside my url polyfill)

Anyhow I will probably update the PR tomorrow.

[ai]

Yeap, disabling this features will be the better option.

[barak007]

It turned out more "complex" for me to remove these features because I wasn't sure how to handle the error cases.

Should I add // istanbul ignore next on these or try to invent a test method for these?

[ai]

Should I add // istanbul ignore next on these or try to invent a test method for these?

Yeap

[ai]

Looks good. Thanks.

I will release it next week since I need to take deal with another issue.

[barak007]

Thanks.

[ai]

Do you have Twitter to mention you in the release tweet?

[barak007]

No Twitter 10x.