Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(builtin): new js_library rule #2109

Merged
merged 47 commits into from
Aug 17, 2020
Merged

Conversation

mistic
Copy link
Contributor

@mistic mistic commented Aug 12, 2020

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Currently the js_library rule is very limited and is not built to be public. Additionally node_module_library was used only for internal purposes in order to represent the npm packages built upon yarn_install/npm_install.

Issue Number: #1967 , #1896 , #149

What is the new behavior?

That PR introduces a new design for a new js_library that could replace both the legacy rules and that is supposed to act as the default/standard rule when you want to share first party libraries across the repository.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

I've considered that PR doesn't include any breaking change as both node_module_library and js_library are not public rules available to be widely used out of this repo.

PS: For now I'm opening the PR as a draft one to collect feedback from @alexeagle and @mrmeku

@alexeagle
Copy link
Collaborator

/cc @mrmeku

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woohoo!!

I wonder if we want to add an example of how this is used, could be in a separate PR.

I like that you haven't exposed this as public API yet, so we can still iterate on it even after next release.

internal/js_library/js_library.bzl Outdated Show resolved Hide resolved
packages/terser/test/inline_sourcemap/input.js Outdated Show resolved Hide resolved
internal/js_library/js_library.bzl Outdated Show resolved Hide resolved
internal/js_library/js_library.bzl Outdated Show resolved Hide resolved
internal/js_library/js_library.bzl Show resolved Hide resolved
internal/js_library/js_library.bzl Outdated Show resolved Hide resolved
@mistic
Copy link
Contributor Author

mistic commented Aug 13, 2020

@alexeagle yeah I believe an example should be added in a separate PR as soon as we finish the design of this rule and got the CI to pass!

I like that you haven't exposed this as public API yet, so we can still iterate on it even after next release.

The idea is to make it public once we got around all the details and we feel confident about it.

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is looking great! Let's promote it out of draft and get merged.

third_party/github.com/gjtorikian/isBinaryFile/BUILD.bazel Outdated Show resolved Hide resolved
third_party/github.com/gjtorikian/isBinaryFile/BUILD.bazel Outdated Show resolved Hide resolved
internal/js_library/js_library.bzl Outdated Show resolved Hide resolved
internal/js_library/js_library.bzl Show resolved Hide resolved
@mistic mistic marked this pull request as ready for review August 14, 2020 20:37
@mistic mistic changed the title feat(builtin): new js_library rule proposal feat(builtin): new js_library rule Aug 14, 2020
@mistic
Copy link
Contributor Author

mistic commented Aug 14, 2020

I've promoted it to the state ready for review and I'll work and close all the feedback in the next couple of hours.

@mistic
Copy link
Contributor Author

mistic commented Aug 15, 2020

@alexeagle I think I just need to remove the changes I've introduced on the files with copy_to_bin and we should be good to go. Will finish it tomorrow

# logic on js_library will consider this a
# node_module when used in examples/user_managed_deps
# which will endup failing as we cannot use node_modules
# from multiple workspaces.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing contribution, thank you for following through!

/cc @mrmeku

@alexeagle alexeagle merged commit 4fe1a17 into bazelbuild:stable Aug 17, 2020
mgenov pushed a commit to mgenov/rules_nodejs that referenced this pull request Jan 6, 2021
Removes `node_module_library` private API and renames it to `js_library`

the latter will be promoted to a public API in a followup change
lherman-cs pushed a commit to lherman-cs/rules_nodejs that referenced this pull request Jan 15, 2021
fix: remove duplicate Importing

chore: upgrade rollup to ^2.3.0

fix: coverage (bazelbuild#2100)

* test: add coverage e2e test

* fix(builtin): fix coverage lcov merger script jumping out of sandbox and not being able to find c8

* test: don't test coverage on Windows

docs: use @npm//@bazel for example load sites

fix(cypress): allow for async cypress plugins

cypress_repository now fails if cypress verify fails
set the HOME env variable since cypress writes files to it
remove unused includeScreenshots / includeVideos variables
browserify files are no longer included in test output files
screenshots/videos are stored directory in TEST_UNDECLARED_OUTPUTS_DIR

docs: update docs for release

chore(release): 2.0.2

chore: update lock files for release

feat(example): add targets in angular_bazel_architect for production serve and build

fix(examples): use ./ prefix on babel config file

Otherwise if it's in a subfolder, babel will think it's an npm package

chore: update dependency com_google_protobuf to v3.13.0 (bazelbuild#2103)

refactor(rollup): use rollup loadConfigFile API

chore: update dependency bazel_toolchains to v3.4.1

build(deps): bump elliptic in /e2e/fine_grained_symlinks

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.4.1 to 6.5.3.
- [Release notes](https://github.com/indutny/elliptic/releases)
- [Commits](indutny/elliptic@v6.4.1...v6.5.3)

Signed-off-by: dependabot[bot] <support@github.com>

fix(typescript): only expect .js outs for .tsx? srcs (bazelbuild#2118)

Fixes bazelbuild#2115

feat(builtin): new js_library rule (bazelbuild#2109)

Removes `node_module_library` private API and renames it to `js_library`

the latter will be promoted to a public API in a followup change

fix(typescript): produce .d.ts as default output rather than empty (bazelbuild#2117)

This lets you type-check a ts_project(emit_declaration_only=True) just by building it (its default outputs)

Fixes bazelbuild#2116

added script to generate NodeJS versions for node_repositories and added most up-to-date versions (bazelbuild#2126)

Co-authored-by: Matt Insler <matt.insler@airbnb.com>

refactor: move node version list into its own file (bazelbuild#2128)

Add a package.json script for updating it

docs: update docs for release

chore(release): 2.0.3

chore: update lock files for release

fix(typescript): add the tsBuildInfoFile option to ts_project (bazelbuild#2138)

Like other options that affect tsc outputs, Bazel needs to know the value from the tsconfig if its set.
Add it to our validator so there's a buildozer command to sync the option value into the BUILD file.

Fixes bazelbuild#2137

refactor(typescript): remove private _TsConfigInfo in ts_project (bazelbuild#2139)

Instead use the same TsConfigInfo that ts_library uses.

Fixes bazelbuild#1931

docs: refresh docsite

docs: minor cleanup of builtins page

don't include the defaults which are giant dictionaries as they render poorly

docs: fix spacing and link colours on rhs sidenav

Fix circleci status badges to pin to stable branch

CircleCI docs claim that they should use the default branch by default, but this icon is green while stable is red, so I think it's still reporting on master.

ci: all angular examples timeout=long

chore: update dependency bazel_toolchains to v3.4.2

chore: update circleci image to latest to pick up newer chrome

docs: add search to docsite using gcse

feat(typescript): generate tsconfig.json for ts_project (bazelbuild#2130)

This is an experimental, opt-in feature where ts_project will generate a tsconfig.json file in place of
the user specifying one in the source directory.

I expect a long tail of compatibility bugs since any path appearing in this generated file needs to point from
bazel-out back to the source directory.

Fixes bazelbuild#2058

docs: replace references to //packages/ with the @npm//@bazel/ equivalent (bazelbuild#2154)

feat(builtin): accept any stamp vars in pkg_npm

NOTE: instead of replacing the replace_with_version with 0.0.0 in unstamped builds,
we now omit the replacement.

See bazelbuild#1694

docs(rollup): add stamping and debug to rollup doc

Also move most of the content above the list of rules. It shows up better in the TOC since rollup_bundle
can be its own entry later in the page, and avoids the API doc table of attributes just appearing with no header.

docs: document the node_context_data attribute

It can be useful if you need to turn on/off stamping per-target.
`java_binary` gives a dedicated stamp attribute for this purpose

Fixes bazelbuild#1693

chore: hide generated .html files in code review

feat(builtin): support for substitutions

fix: use golden_file_test instead

refactor: cleanup pkg_web stamping feature

chore: update dependency bazel_toolchains to v3.5.0 (bazelbuild#2168)

Updated nodejs versions (14.9.0) (bazelbuild#2169)

docs: update docs for release

chore(release): 2.1.0

chore: update lock files for release

chore: try github actions version of stale bot

The probot one never triggered despite being setup correctly AFAICT.
This bot seems better supported and I sow it working on bazel-watcher repo.

build(deps): bump http-proxy from 1.18.0 to 1.18.1

Bumps [http-proxy](https://github.com/http-party/node-http-proxy) from 1.18.0 to 1.18.1.
- [Release notes](https://github.com/http-party/node-http-proxy/releases)
- [Changelog](https://github.com/http-party/node-http-proxy/blob/master/CHANGELOG.md)
- [Commits](http-party/node-http-proxy@1.18.0...1.18.1)

Signed-off-by: dependabot[bot] <support@github.com>

build(deps): bump yargs-parser in /e2e/ts_devserver

Bumps [yargs-parser](https://github.com/yargs/yargs-parser) from 13.1.1 to 13.1.2.
- [Release notes](https://github.com/yargs/yargs-parser/releases)
- [Changelog](https://github.com/yargs/yargs-parser/blob/master/docs/CHANGELOG-full.md)
- [Commits](https://github.com/yargs/yargs-parser/commits)

Signed-off-by: dependabot[bot] <support@github.com>

ci: stamp our releases with a stable key

fix(builtin): don't set --preserve-symlinks-main by default (bazelbuild#2176)

feat: add link_workspace_root to nodejs_binary, npm_package_bin, rollup_bundle, terser_minified, ts_project

Link the workspace root to the bin_dir to support absolute requires like 'my_wksp/path/to/file'.
If source files need to be required then they can be copied to the bin_dir with copy_to_bin.

fix(builtin): fix bazel coverage masking test failures

test(builtin): test failing test with bazel coverage

build(deps): bump http-proxy in /examples/protocol_buffers

Bumps [http-proxy](https://github.com/http-party/node-http-proxy) from 1.18.0 to 1.18.1.
- [Release notes](https://github.com/http-party/node-http-proxy/releases)
- [Changelog](https://github.com/http-party/node-http-proxy/blob/master/CHANGELOG.md)
- [Commits](http-party/node-http-proxy@1.18.0...1.18.1)

Signed-off-by: dependabot[bot] <support@github.com>

build(deps): bump http-proxy in /examples/web_testing

Bumps [http-proxy](https://github.com/http-party/node-http-proxy) from 1.18.0 to 1.18.1.
- [Release notes](https://github.com/http-party/node-http-proxy/releases)
- [Changelog](https://github.com/http-party/node-http-proxy/blob/master/CHANGELOG.md)
- [Commits](http-party/node-http-proxy@1.18.0...1.18.1)

Signed-off-by: dependabot[bot] <support@github.com>

test(rollup): fix test failing due to missing dep

fix(rollup): allow config files to override default onwarn method

Fixes bazelbuild#2084

feat: link_workspace_root not needed in terser_minified

docs: add spica to adopter organization list (bazelbuild#2184)

* Add spica to adopter organization list (bazelbuild#2183)
* docs: update the 'send a PR' link to point to the stable branch

Co-authored-by: Sahin Yort <thesayyn@gmail.com>

feat: promote js_library to public API

Fixes bazelbuild#149
Fixes bazelbuild#1771

docs: update docs for release

chore(release): 2.2.0

chore: update lock files for release

Update readme (bazelbuild#2190)

Fix missing `}` and indentation

feat: add strict_visibility to npm_install / yarn_install rules (bazelbuild#2193)

With strict_visibility enabled (currently defaults false), only dependencies within the given `package.json` file are given public visibility.

All transitive dependencies are given limited visibility, enforcing that all direct dependencies are listed in the `package.json` file.

closes bazelbuild#2110

fix: don't glob yarn or node files when using vendored_node or vendored_yarn

- Fixes a visibility issue with rollup-worker

fix(examples): prevent ibazel EOF

Fixes bazelbuild#2143

docs(karma): document how to use non-headless Chrome (bazelbuild#2202)

Fixes bazelbuild#1881

docs(labs): produce stardoc API for labs

Fixes bazelbuild#2195

fix(karma): allow custom browsers to specify args (fixes bazelbuild#595)

Today, the args as specified in the various manifests of the rules_webtesting
browsers never make it into the generated karma.conf.js. This results in these
arguments never being used when launching Chrome, preventing customization of
browsers such as window size, enabling remote debugging, or other flag-based
options. This PR fixes this by reading those arguments from the manifest, and
adding them to the browsers list in the generated karma.conf.js.

Also, this PR includes a change to generated_file_test to allow a golden file
to represent a substring of the generated output.

Also Also: This PR includes a golden file test that verified that the generated
karma.conf.js does read in the specified value.

Furthermore, the effect of this can be verified manually via:
```
VERBOSE_LOGS=1 bazel run packages/karma/test/karma:testing_custom_chrome
```
Note the appearance of the additional flags in the new debug output.

fix(builtin): js_library: correctly propagate DeclarationInfos

Transitive typings are added to typings_depsets, but we were checking
the typings array instead.

Signed-off-by: Duarte Nunes <duarte@hey.com>

feat: update nodejs versions (bazelbuild#2207)

include 10.22.1, 12.18.4, 14.10.0, 14.10.1, 14.11.0 and 14.12.0 in node_versions

Added s390x support

docs: update docs for release

chore(release): 2.2.1

chore: update lock files for release

ci: tell Angular CLI not to update webdriver

It can get ahead of the Chrome version on our CI machine

ci: replace --webdriver-update=false with --no-webdriverUpdate

Architect CLI uses minimist under the hood to parse arguments. Minimist will not convert boolean like values unless they are known options or the `{boolean: true}` option is configured (Which is not in Architect CLI).

feat(karma): use Trusted Types policy when loading scripts for Karma

When the Karma plugin is used in a testing environment that enforces
Trusted Types, its loadFile functionality currently fails due to a
Trusted Types violation when assigning to script.textContent. This makes
it impossible to use the plugin with integration tests that ensure an
application is compatible with Trusted Types.

This is fixed by creating a Trusted Types policy specifically for the
Karma plugin, and use it to promote any loaded scripts to a TrustedScript
before assigning to script.textContent. This is done in a way that is
backwards compatible:
- The policy is `null` in browsers that don't yet support Trusted Types,
  in which case the original script string is used as before.
- When Trusted Types are supported in the browser but not enforced by
  the application, the browser treats the TrustedScript as if it were a
  string when it is assigned to script.textContent.

chore: update dependency bazel_toolchains to v3.6.0

docs: remove HTML tables so docs can be authored in markdown without need for html escaping

docs(rollup): improve clarity

feat(example): add full pwa support

feat(example): service worker update handling

fix(exmaple): add docstring to ngsw_config rule

fix(example): remove server side compression

fix(example): remove index.html from prodapp srcs

fix(example): remove compression dependencies

fix(builtin): js_library supports --output_groups=types

If providing DeclarationInfo we should also give this way for filegroup/bazel CLI to select them

chore: docgen

docs: update docs for release

chore(release): 2.2.2

chore: update lock files for release

fix(builtin): give a longer timeout for _create_build_files

A user reports that their build file generation takes longer than 600 seconds.
It's probably a bug that we are so slow, but let's address that separately (maybe in our work for npm v7)

Also note we could have made this timeout user-configurable, but I think the extra API surface isn't worth the benefit.

Fixes bazelbuild#2231

fix(typescript): don't include _valid_options marker file in outs

Tested by building //packages/angular:npm_package and don't get an extra .d.ts file in the package.
Also tested the negative case by making an invalid option and check that it's still reported.

Fixes bazelbuild#2078

chore: allow node versions <= 14.14.0

chore: update rules_docker to 0.14.4

docs: minor update for readability and formatting (bazelbuild#2243)

* docs: minor update for readibility and formating

Fix small typo in changelog

chore(docs): move ts_library docs into ts_library

Also recommend ts_project for new uses

chore(karma): remove dependency on tmp

feat(typescript): add allow_js support to ts_project

fix(exmaples/nestjs): add module_name field in ts_library

docs: fix a typo of angular-architect example (bazelbuild#2272)

chore: bump days before closing stale issues to 90

Fix select with conditons on darwin.

Bazel is changing how @bazel_tools/conditions:darwin is implemented.
With old implementation based on flags, it was ok to have darwin and
darwin_x86_64 in a select. This was based on flag --cpu=darwin and
--cpu=dawin_x86_64.

New implementation is based on constraints, where "darwin" means OS (and
any CPU), while "darwin_x86_64" mean only specific CPU. As such they
cannot be used in the same select, because the selection would be
ambiguous.

build: add 'fix-windows' tag due to windows flaky tests

Will revisit the linker and test, but this is blocking PRs from being merged

feat(typescript): worker mode for ts_project (bazelbuild#2136)

* feat(typescript): worker mode for ts_project

* chore: cleanup and declare protobufjs dependency

* fix: do not pass --watch for non-worker mode

* fix: do not test worker on windows

* fix: log on standalone failures

* chore: docs improvements

Co-authored-by: Alex Eagle <alex.eagle@robinhood.com>
Co-authored-by: Dan Muller <mrmeku@stairwell.com>

feat(node_repositories): Added auth option for downloading nodejs and yarn

fix: npm_package.pack should work in windows os

feat(examples): adds example for running jest with typescript (bazelbuild#2245)

fix(typescript): specify rootDir as absolute path

Also allow non-ts files across src/bin dir, since they are not emitted we don't need the rootdir
calculation to take them into account. This lets the Angular compiler accept .ts sources from
source dir even if the css inputs are generated.

refactor: ts_project cleanup

feat(cypress): remove browiserify preprocessor

docs: update docs for release

chore(release): 2.3.0

chore: update lock files for release

fix: npm_package.pack on Windows should not generate undefined.tgz

chore: update nodejs versions (bazelbuild#2284)

perf(cypress): pack cypress runfiles into a single tar

chore: add GitHub Workflow to update NodeJS versions every night

chore: fix a link

Current link returns 404, it seems that the url was somehow changed to .html from .md.

chore: fix the link again

haha

fix(builtin): make linker deterministic when resolving from manifest & fix link_workspace_root with no runfiles

chore: re-lock jest dependencies

It was using a non-standard repository, which had downtime this morning

chore(release): 2.3.1

chore: update lock files for release

chore: update dependency com_google_protobuf to v3.14.0

chore: update dependency bazel_toolchains to v3.7.0

fix launcher script terminating before child terminates on SIGTERM

chore: update dependency bazel_toolchains to v3.7.1

fix(examples): fix jest example on windows

Fixes bazelbuild#1454

fix(builtin): give better error when linker runs on Node <10

Example how this looks now:

```
[20 / 21] Bundling JavaScript bundle.js [parcel]; 4s remote-cache, darwin-sandbox
ERROR: /private/var/folders/r7/2kp53v7n091gcz93xlt7wtd80000gn/T/tmp-48948DHy4HrXTwhRy/BUILD.bazel:4:1: Bundling JavaScript bundle.js [parcel] failed (Exit 1) parcel.sh failed: error executing command bazel-out/host/bin/external/npm/parcel-bundler/bin/parcel.sh build foo.js --out-dir bazel-out/darwin-fastbuild/bin --out-file bundle.js ... (remaining 2 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
ERROR: rules_nodejs linker requires Node v10 or greater, but is running on 8.11.2
Note that earlier Node versions are no longer in long-term-support, see
https://nodejs.org/en/about/releases/
INFO: Elapsed time: 40.360s, Critical Path: 12.97s
INFO: 0 processes.
FAILED: Build did NOT complete successfully
//:test                                                               NO STATUS
```

Fixes bazelbuild#2304

docs(typescript): link some user-contributed doc (bazelbuild#2322)

Fixes bazelbuild#2298

chore: allow node versions >=12 <=14 (bazelbuild#2332)

chore: allow node versions >=12 <=14

feat: add esbuild package

chore: add module_mapping support via the aspect instead of manual implementing it

chore: sqash

feat: update to latest esbuild, add additional flags when not minifying

fix: consume both JSEcmaScriptModuleInfo and JSModuleInfo to support js_library

fix: add missing bzl libraries and publish helpers

test: use runfiles helper in test

fix: use exe for Windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants