Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

Release to be compatible with NodeJS 14+? #73

Open
dgp1130 opened this issue Aug 28, 2021 · 11 comments
Open

Release to be compatible with NodeJS 14+? #73

dgp1130 opened this issue Aug 28, 2021 · 11 comments

Comments

@dgp1130
Copy link
Contributor

dgp1130 commented Aug 28, 2021

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 #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:

  • Remove sourcemap = True, which is a functionality regression.
  • Revert to an older version of NodeJS, v13 is already out of maintenance, but v12 is still in LTS, so I think 12.22.5 is the current latest supported version that works with @bazel/postcss@0.5.0.
  • Use rules_postcss via a repository rule linked against 95fefe3 or later. This is highly discouraged and a lot of effort to work around a simple bug.
  • Locally patch @bazel/rules_postcss to include 95fefe3. Also a lot of effort for a simple bug.

Any chance we can get a release with this fix?

dgp1130 added a commit to dgp1130/rules_prerender that referenced this issue Aug 28, 2021
…`4.0.0`, and NodeJS to `12.22.5`.

These are the latest Bazel and `rules_nodejs` versions, necessary to include some fixes/improvements for re-exporting `js_library()` and `ts_library()` targets, see #39 (comment).

`rules_nodejs@4.0.0` actually updates NodeJS to `14.17.5`, however this breaks `@bazel/postcss@0.5.0` and there is no later release for it. There is actually a fix, but it hasn't been released. I filed bazelbuild/rules_postcss#73 to request a release, but for now I'm keeping NodeJS at `12.22.5`, which is the current latest release that does not include the v14 breaking change. Note that NodeJS v13 is already unsupported as it is not an LTS release.

This is the most I can upgrade Bazel/`rules_nodejs` without breaking `@bazel/postcss`.
dgp1130 added a commit to dgp1130/rules_prerender that referenced this issue Aug 28, 2021
…`4.0.0`, and NodeJS to `12.22.5`.

Refs #39.

These are the latest Bazel and `rules_nodejs` versions, necessary to include some fixes/improvements for re-exporting `js_library()` and `ts_library()` targets, see #39 (comment).

`rules_nodejs@4.0.0` actually updates NodeJS to `14.17.5`, however this breaks `@bazel/postcss@0.5.0` and there is no later release for it. There is actually a fix, but it hasn't been released. I filed bazelbuild/rules_postcss#73 to request a release, but for now I'm keeping NodeJS at `12.22.5`, which is the current latest release that does not include the v14 breaking change. Note that NodeJS v13 is already unsupported as it is not an LTS release.

This is the most I can upgrade Bazel/`rules_nodejs` without breaking `@bazel/postcss`.
@nex3
Copy link
Contributor

nex3 commented Aug 31, 2021

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

@rzhw
Copy link
Contributor

rzhw commented Aug 31, 2021

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
Copy link

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
Copy link
Contributor

rzhw commented Aug 31, 2021

@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
Copy link
Contributor Author

dgp1130 commented Aug 31, 2021

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

@dgp1130
Copy link
Contributor Author

dgp1130 commented Sep 4, 2021

@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 (30d1ba5) or when the @bazel/worker dep was moved from dependencies to devDependencies (cad4b36) 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
Copy link

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

@rzhw
Copy link
Contributor

rzhw commented Sep 6, 2021

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

@dgp1130
Copy link
Contributor Author

dgp1130 commented May 6, 2022

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
Copy link

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
Copy link

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.

dgp1130 added a commit to dgp1130/rules_prerender that referenced this issue Feb 8, 2023
Refs #46.

This is blocking the Node.js upgrade past v12 and the feature doesn't really work anyways as it isn't currently linked properly in the output. See: bazelbuild/rules_postcss#73.

Hopefully a future migration to Parcel will put us back in a position where CSS sourcemaps can be used.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants