search

bazelbuild / rules_postcss

PostCSS rules for Bazel
Apache License 2.0
10 stars 13 forks source link

Release to be compatible with NodeJS 14+? #73

Open dgp1130 opened 3 years ago

dgp1130 commented 3 years ago

rules_nodejs@4.0.0 uses NodeJS 14.17.5 by default. This is not compatible with @bazel/postcss@0.5.0 as it will not generate source maps. Attempting to use postcss_binary(sourcemap = True) fails with:

(node:38) UnhandledPromiseRejectionWarning: TypeError [ERR_INVALID_ARG_TYPE] [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received an instance of SourceMapGenerator
    at Object.writeFileSync (fs.js:1517:5)
    at /home/dparker/.cache/bazel/_bazel_dparker/3b4ba8c4eec5951318c36cb07368e75b/sandbox/linux-sandbox/1223/execroot/rules_prerender/bazel-out/host/bin/examples/styles/page_styles.postcss_runner.runner_src.js:213:16
(Use `node --trace-warnings ...` to show where the warning was created)
(node:38) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:38) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
ERROR: /home/dparker/Source/rules_prerender/examples/styles/BUILD.bazel:5:16: output 'examples/styles/page_styles_bundled.css.map' was not created
ERROR: /home/dparker/Source/rules_prerender/examples/styles/BUILD.bazel:5:16: Running PostCSS runner on <generated file examples/styles/page_page_styles.css> failed: not all outputs were created or valid

This appears to have been fixed a couple months ago in https://github.com/bazelbuild/rules_postcss/pull/69, which solved the problem in google3, but this doesn't appear to have been released to NPM. Considering that rules_nodejs@4 uses Node 14 by default, @bazel/postcss is not usable out of the box and requires an older Node version to run.

Workarounds include:

Any chance we can get a release with this fix?

nex3 commented 3 years ago

I'm not sure who publishes the npm package. @alexeagle is it you?

rzhw commented 3 years ago

If this is referring to @bazel/postcss that's me at the moment, apologies there hasn't been a release in a while.

I'll resolve this.

alexeagle commented 3 years ago

hey while we're chatting maintenance, @rzhw I'm curious what your experience has been maintaining a ruleset outside of the rules_nodejs monorepo. We are starting planning for our next major release and I'm considering whether we could reduce scope of the monorepo and follow this model for new rules (Jest, Webpack seem likely soon) and possibly fork out some that are currently in there. Anything you would do differently now that you've been operating this for a while?

rzhw commented 3 years ago

@dgp1130 I just released @bazel/postcss@0.6.0, let me know if that doesn't work for you.

@alexeagle

It's been pretty smooth so far with the current setup we have here, but I think a lot of that can be owed to how these external rules aren't under active heavy development, nor is there any sizeable user base at the moment. I'm sure that if that wasn't the case, things I might want to revisit might become lot more obvious.

So far looking back, I think the only thing I might revisit (but still have the opportunity to revisit) is whether the dual Bazel-style and Node-style distribution still makes sense, but it doesn't exactly add much overhead either. Not that big of a deal at the moment.

Otherwise I was concerned about needing to follow rules_nodejs version bumps as well. But I suppose if you're spinning out rules from the monorepo there'll be a bunch of new repos that encounter this same problem, and it'll be good to be in a crowd for these kinds of problems rather than staking it out alone as has been the case thus far :)

Plus not being able to rely on things like common monorepo scripts for publishing. If things have to be done in a non-standard way, having it be done through a centralised place is pretty nice. Using pkg_npm for publishing instead of using the npm cli seems slightly... painful.

Just now I ended up manually doing a bazel run :npm_package.pack and npm publish bazel-postcss-0.6.0.tgz, because for whatever reason bazel run :npm_package.publish -- --access=public as I've done previously ended up spitting out a npm ERR! 404 '@bazel/postcss@0.6.0' is not in the npm registry. error.

Overall I'm pretty happy that this repo works as it has thus far.

dgp1130 commented 3 years ago

Thanks @rzhw! I'll try it out as soon as I get a chance.

dgp1130 commented 3 years ago

@rzhw, unfortunately it seems that a dependency on an old @bazel/worker prevents this from being used with rules_nodejs@4.0.0.

$ bazel test //...
INFO: Invocation ID: 872c2913-ba10-404a-a63e-00f98f7c0842
INFO: Repository npm instantiated at:
  /home/dparker/Source/rules_prerender/WORKSPACE.bazel:26:12: in <toplevel>
  /home/dparker/.cache/bazel/_bazel_dparker/3b4ba8c4eec5951318c36cb07368e75b/external/build_bazel_rules_nodejs/index.bzl:78:17: in npm_install
Repository rule npm_install defined at:
  /home/dparker/.cache/bazel/_bazel_dparker/3b4ba8c4eec5951318c36cb07368e75b/external/build_bazel_rules_nodejs/internal/npm_install/npm_install.bzl:748:30: in <toplevel>
ERROR: An error occurred during the fetch of repository 'npm':
   Traceback (most recent call last):
        File "/home/dparker/.cache/bazel/_bazel_dparker/3b4ba8c4eec5951318c36cb07368e75b/external/build_bazel_rules_nodejs/internal/npm_install/npm_install.bzl", line 725, column 13, in _npm_install_impl
                fail("npm_install failed: %s (%s)" % (result.stdout, result.stderr))
Error in fail: npm_install failed: 
> @bazel/rollup@4.0.0 postinstall /home/dparker/Source/rules_prerender/node_modules/@bazel/rollup
> node npm_version_check.js

> protobufjs@6.8.8 postinstall /home/dparker/Source/rules_prerender/node_modules/protobufjs
> node scripts/postinstall

> @bazel/worker@2.3.3 postinstall /home/dparker/Source/rules_prerender/node_modules/@bazel/worker
> node npm_version_check.js

 (npm WARN prepare removing existing node_modules/ before installation
/home/dparker/Source/rules_prerender/node_modules/@bazel/worker/npm_version_check.js:17
  throw new Error(`Expected package major version to equal @build_bazel_rules_nodejs major version
  ^

Error: Expected package major version to equal @build_bazel_rules_nodejs major version
    @bazel/worker - 2.3.3  
    @build_bazel_rules_nodejs - 4.0.0
  See https://github.com/bazelbuild/rules_nodejs/wiki/Avoiding-version-skew
    at Object.<anonymous> (/home/dparker/Source/rules_prerender/node_modules/@bazel/worker/npm_version_check.js:17:9)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @bazel/worker@2.3.3 postinstall: `node npm_version_check.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @bazel/worker@2.3.3 postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/dparker/.npm/_logs/2021-09-04T20_21_32_397Z-debug.log
)
ERROR: Error fetching repository: Traceback (most recent call last):
        File "/home/dparker/.cache/bazel/_bazel_dparker/3b4ba8c4eec5951318c36cb07368e75b/external/build_bazel_rules_nodejs/internal/npm_install/npm_install.bzl", line 725, column 13, in _npm_install_impl
                fail("npm_install failed: %s (%s)" % (result.stdout, result.stderr))
Error in fail: npm_install failed: 
> @bazel/rollup@4.0.0 postinstall /home/dparker/Source/rules_prerender/node_modules/@bazel/rollup
> node npm_version_check.js

> protobufjs@6.8.8 postinstall /home/dparker/Source/rules_prerender/node_modules/protobufjs
> node scripts/postinstall

> @bazel/worker@2.3.3 postinstall /home/dparker/Source/rules_prerender/node_modules/@bazel/worker
> node npm_version_check.js

 (npm WARN prepare removing existing node_modules/ before installation
/home/dparker/Source/rules_prerender/node_modules/@bazel/worker/npm_version_check.js:17
  throw new Error(`Expected package major version to equal @build_bazel_rules_nodejs major version
  ^

Error: Expected package major version to equal @build_bazel_rules_nodejs major version
    @bazel/worker - 2.3.3  
    @build_bazel_rules_nodejs - 4.0.0
  See https://github.com/bazelbuild/rules_nodejs/wiki/Avoiding-version-skew
    at Object.<anonymous> (/home/dparker/Source/rules_prerender/node_modules/@bazel/worker/npm_version_check.js:17:9)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @bazel/worker@2.3.3 postinstall: `node npm_version_check.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @bazel/worker@2.3.3 postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/dparker/.npm/_logs/2021-09-04T20_21_32_397Z-debug.log
)
ERROR: no such package '@npm//@bazel/postcss': npm_install failed: 
> @bazel/rollup@4.0.0 postinstall /home/dparker/Source/rules_prerender/node_modules/@bazel/rollup
> node npm_version_check.js

> protobufjs@6.8.8 postinstall /home/dparker/Source/rules_prerender/node_modules/protobufjs
> node scripts/postinstall

> @bazel/worker@2.3.3 postinstall /home/dparker/Source/rules_prerender/node_modules/@bazel/worker
> node npm_version_check.js

 (npm WARN prepare removing existing node_modules/ before installation
/home/dparker/Source/rules_prerender/node_modules/@bazel/worker/npm_version_check.js:17
  throw new Error(`Expected package major version to equal @build_bazel_rules_nodejs major version
  ^

Error: Expected package major version to equal @build_bazel_rules_nodejs major version
    @bazel/worker - 2.3.3  
    @build_bazel_rules_nodejs - 4.0.0
  See https://github.com/bazelbuild/rules_nodejs/wiki/Avoiding-version-skew
    at Object.<anonymous> (/home/dparker/Source/rules_prerender/node_modules/@bazel/worker/npm_version_check.js:17:9)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @bazel/worker@2.3.3 postinstall: `node npm_version_check.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @bazel/worker@2.3.3 postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/dparker/.npm/_logs/2021-09-04T20_21_32_397Z-debug.log
)
INFO: Elapsed time: 16.978s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)
FAILED: Build did NOT complete successfully (0 packages loaded)

Tracking the dependency it's definitely coming from @bazel/postcss.

$ npm ls @bazel/worker@2.3.3
rules_prerender@0.0.0-PLACEHOLDER /home/dparker/Source/rules_prerender
└─┬ @bazel/postcss@0.6.0
  └── @bazel/worker@2.3.3

I'm guessing this was introduced either when worker support was added (30d1ba5789f812e00532eb69abf4bbfcf29f44fe) or when the @bazel/worker dep was moved from dependencies to devDependencies (cad4b36ce0a5dc7e2d018dab6614416eb4771702) Both of which seem to be included in 0.6.0 but weren't present in 0.5.0.

I think we either need to upgrade this dep to ^4.0.0 to be compatible with latest rules_nodejs, or we can make it a peer dep and let the consumer install the relevant version. Not sure what is more appropriate here?

alexeagle commented 3 years ago

We relaxed that constraint in the last 3.x release of @bazel/worker just for this reason.

rzhw commented 3 years ago

I'll take the approach of upgrading the rules_nodejs dep. (#74)

dgp1130 commented 2 years ago

Just a heads up I believe Node v12 is now EOL and no longer supported by the Node.js team and rules_postcss is still blocked from upgrading past that. So I think it's impossible to use this repository with any still-supported versions of Node.

alexeagle commented 2 years ago

Maybe it's a good idea to archive this repo to set user expectations? You can always unarchive if there's some new energy to maintain it.

sventiffe commented 2 years ago

Alex, we're currently reviewing all repos under bazelbuild and marked this repo as potentially abandoned half a year ago. We share your conclusion and will archive it and keep it archived until there is someone who would like to take the lead.