Commit Graph

7 Commits

Author SHA1 Message Date
Philipp Schrader 7106d86685
Remove npm/rules_js dependency for C++ only use cases (#7990)
When flatbuffers is being used from a project that has no use for
JavaScript, users encounter an error similar to the following:

    ERROR: Skipping '@com_github_google_flatbuffers//:flatbuffers': error loading package '@com_github_google_flatbuffers//': Unable to find package for @npm//:defs.bzl: The repository '@npm' could not be resolved: Repository '@npm' is not defined.
    WARNING: Target pattern parsing failed.
    ERROR: error loading package '@com_github_google_flatbuffers//': Unable to find package for @npm//:defs.bzl: The repository '@npm' could not be resolved: Repository '@npm' is not defined.
    INFO: Elapsed time: 0.023s
    INFO: 0 processes.
    FAILED: Build did NOT complete successfully (0 packages loaded)
        currently loading: @com_github_google_flatbuffers//

That's not ideal. Users that only care about C++ for example
shouldn't be forced to deal with rules_js and friends.

This patch attempts to fix that by moving the rules_js-specific things
into the `ts` and `tests/ts` directories. This should allow
non-JavaScript projects to ignore rules_js and friends completely.

Here I basically followed the `rules_foo` example from rules_js:
https://github.com/aspect-build/rules_js/tree/main/e2e/rules_foo

The idea is that flatbuffers has its own npm dependencies regardless
of what other projects may have. This means we should not force the
user to import flatbuffers's npm dependencies. The new
`ts/repositories.bzl` file is used by dependents to import
flatbuffers's dependencies. They can still import their own
dependencies. This cleanup allowed me to move all
JavaScript-specific stuff from the top-level directory into
subdirectories.

There should be no changes in this patch in terms of functionality.
It's just a refactor of the rules_js call sites. Users will have to
add a call to the function in `ts/repositories.bzl` in their own
`WORKSPACE` file. They can use
`tests/ts/bazel_repository_test/WORKSPACE` as an example.

Co-authored-by: Derek Bailey <derekbailey@google.com>
2024-04-18 05:06:06 +00:00
Philipp Schrader 362dd663f8
Fix BUILD.bazel style violations (#8081)
`buildifier` was complaining as follows:

    #### :bazel: buildifier: found 2 lint issues in your WORKSPACE, BUILD and *.bzl files
    <pre><code>tests/ts/bazel_repository_test_dir/BUILD:3:1: <a href="https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#out-of-order-load">out-of-order-load</a>: Load statement is out of its lexicographical order.
    ts/BUILD.bazel:2:1: <a href="https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#out-of-order-load">out-of-order-load</a>: Load statement is out of its lexicographical order.</pre></code>

This can be fixed locally like so:

    $ buildifier -lint fix $(git ls-files | grep -e '/BUILD.bazel$' -e '/BUILD$' -e '\<WORKSPACE$')

I also took this opportunity to fix one of the filenames.

I accidentally introduced these errors in #8078.
2023-08-28 09:20:10 +02:00
Philipp Schrader c1e7aee489
Migrate from rules_nodejs to rules_js/rules_ts (take 2) (#7928)
* Migrate from rules_nodejs to rules_js/rules_ts (take 2)

This is the second version of patch #7923. The first version got
reverted because bazel query was failing:

    $ bazel --nosystem_rc --nohome_rc query tests(set('//...')) except tests(attr("tags", "manual", set('//...')))
    ERROR: Traceback (most recent call last):
    	File "/workdir/tests/ts/bazel_repository_test_dir/BUILD", line 6, column 22, in <toplevel>
    		npm_link_all_packages(name = "node_modules")
    	File "/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/ec321eb2cc2d0f8f91b676b6d4c66c29/external/npm/defs.bzl", line 188, column 13, in npm_link_all_packages
    		fail(msg)
    Error in fail: The npm_link_all_packages() macro loaded from @npm//:defs.bzl and called in bazel package 'tests/ts/bazel_repository_test_dir' may only be called in bazel packages that correspond to the pnpm root package '' and pnpm workspace projects ''

This was happening because the `.bazelrc` file only added
`--deleted_packages` to the `build` command. We also need it for the
`query` command. This second version of the patch fixes that.

Original commit message:

This patch migrates the current use of rules_nodejs to the new rules_js.
rules_js is the intended replacement of rules_nodejs as per this note:
https://github.com/aspect-build/rules_js#relationship-to-rules_nodejs

> rules_js is an alternative to the build_bazel_rules_nodejs Bazel module
> and accompanying npm packages hosted in
> https://github.com/bazelbuild/rules_nodejs, which is now
> unmaintained. All users are recommended to use rules_js instead.

There are a few notable changes in this patch:
1. The `flatbuffer_ts_library` macro no longer accepts a `package_name`
   attribute. This is because rules_js appears to manage the import
   naming of dependencies via top-level `npm_link_package` targets.
   Users will have to migrate.
2. I added a few more arguments to `flatbuffer_library_public()`. These
   helped with exposing esbuild to `ts/compile_flat_file.sh`.
3. I pinned the version of `typescript` in `package.json` so that
   rules_ts can download the exact same version. rules_ts doesn't know
   what to do if the version isn't exact.
4. Since rules_js uses the pnpm locking mechanism, we now have a
   `pnpm-lock.yaml` file instead of a yarn lock file.
4. I added bazel targets for a few of the existing tests in `tests/ts`.
   They can be run with `bazel test //test/ts:all`. Since there is no
   flexbuffers bazel target, I did not add a bazel target for the
   corresponding test.
5. I added a separate workspace in `tests/ts/bazel_repository_test_dir/`
   to validate that the flatbuffers code can be imported as an external
   repository. You can run the test with
   `bazel test //test/ts:bazel_repository_test`. For this to work, I
   needed to expose a non-trivial chunk of the flatbuffers code to the
   test. I achieved this through some recursive `distribution`
   filegroups. This is inspired by rules_python's workspace tests.

I did not do anything special to validate that the `gen_reflections`
parameter works the same. This patch doesn't change anything about
the TypeScript generation.

As a side note: I am not an expert with rules_js. This patch is my
attempt based on my limited understanding of the rule set.

Fixes #7817

* Fix the query

---------

Co-authored-by: Derek Bailey <derekbailey@google.com>
2023-05-03 11:48:15 -07:00
Derek Bailey e7dc252b0e
Revert "Migrate from rules_nodejs to rules_js/rules_ts (#7923)" (#7927)
This reverts commit 4172c3f0bd.
2023-04-28 12:58:49 -07:00
Philipp Schrader 4172c3f0bd
Migrate from rules_nodejs to rules_js/rules_ts (#7923)
* Start using pnpm

* Add @npm

* get more stuff set up

* Get the analysis phase passing.

* Get esbuild working?

* Get it compiling?

    $ bazel build //tests/ts/...

* Try to get the test working

* test is passing

* Get the other tests working

* clarify comment

* clean up a bit

* Try to add another test

* Add another test

* clean up more

* remove unused reference

* Add e2e test

* Get more of the test working

* add lock file

* Get test working on its own

* Get e2e test passing

* fix infinite recursion

* Add comments

* clean up some more

* clean up more again

* Source typescript version from package.json

* run buildifier

* lint

* Fix unset `extra_env`

* Incorporate feedback

* run buildifier

---------

Co-authored-by: Derek Bailey <derekbailey@google.com>
2023-04-28 18:17:45 +00:00
Björn Harrtell ef76b5ece4
[TS/JS] Entry point per namespace and reworked 1.x compatible single file build (#7510)
* [TS/JS] Entry point per namespace

* Fix handling of outputpath and array_test

* Attempt to fix generate_code

* Fix cwd for ts in generate_code

* Attempt to fixup bazel and some docs

* Add --ts-flat-files to bazel build to get bundle

* Move to DEFAULT_FLATC_TS_ARGS

* Attempt to add esbuild

* Attempt to use npm instead

* Remove futile attempt to add esbuild

* Attempt to as bazel esbuild

* Shuffle

* Upgrade bazel deps

* Revert failed attempts to get bazel working

* Ignore flatc tests for now

* Add esbuild dependency

* `package.json` Include esbuild

* `WORKSPACE` Add fetching esbuild binary

* Update WORKSPACE

* Unfreeze Lockfile

* Update WORKSPACE

* Update BUILD.bazel

* Rework to suggest instead of running external bundler

* Add esbuild generation to test script

* Prelim bundle test

* Run test JavaScriptTest from flatbuffers 1.x

* Deps upgrade

* Clang format fix

* Revert bazel changes

* Fix newline

* Generate with type declarations

* Handle "empty" root namespace

* Adjust tests for typescript_keywords.ts

* Separate test procedure for old node resolution module output

* Fix rel path for root level re-exports

* Bazel support for esbuild-based flatc

Unfortunately, we lose typing information because the new esbuild method
of generating single files does not generate type information.

The method used here is a bit hack-ish because it relies on parsing the
console output of flatc to figure out what to do.

* Try to fix bazel build for when node isn't present on host

* Auto formatting fixes

* Fix missing generated code

Co-authored-by: Derek Bailey <derekbailey@google.com>
Co-authored-by: James Kuszmaul <jabukuszmaul+collab@gmail.com>
2023-01-21 12:22:22 -08:00
James Kuszmaul e5f331db99
[TS] Add single-file ts codegen & bazel rule for typescript (#7161)
The headline here is adding a flatbuffer_ts_library rule for generating
typescript code in bazel. This entails some non-trivial other changes,
but ideally none are user-visible.

In particular:
* Added a --ts-flat-file flag that generates a single *_generated.ts
  file instead of separate files for each typescript type. This makes
  bazel much happier.
* Import the bazel rules_nodejs stuff needed to support building
  typescript in bazel
* Move flatbuffers.ts to index.ts because I wasn't sure how to make
  bazel comprehend the "main" attribute of the package.json. Happy
  to take another stab at figuring that out if really needed.
* Fix another couple keyword escaping spots in typescript...
2022-03-10 10:08:13 -08:00