fixes clean/build cache & brings back files watch #183

OskarDamkjaer · 2024-03-01T13:58:48Z

Now running

npm run build
npm run clean
npm run build
npm run test

works as intended. It seems the turborepo cache restores the outputs of our build steps, so the second build is instant and just re-creates dist folders from its own cache - meaning the clean command doesn't really do anything. If we want it to rebuild, you'd need to use the --force flag, but that should (hopefully) not be needed when the caching is properly set up. Should we remove the clean script and maybe replace it with a force-build?

I added a prelaunch task to build the vscode extension when launching the vscode extension via the sidebar, since the cache now seems to work as intended it only rebuilds what's necessary, this means I can just click the reload button in vscode while developing to see my new changes 😌 I have a vague memory we had this before (?) but prior to this change I had to rebuild everything manually to see the changes in vscode
Edit: the debugger seemed to struggle to attach with my change.. I left it out for now

I also wonder if it's be worth switching from turbo to wireit as we're not thrilled with turborepo so far and it doesn't support watching for file changes, while writeit does. It would be nice with file watching to so that the playgrounds & tests could be automatically reloaded when coding. If the cache issues go away, I'm fine sticking with turborepo for a while longer though. WDYT @ncordon ?

ncordon · 2024-03-18T14:38:02Z

turbo.json

-    "antlr4-c3#build": {
-      "dependsOn": ["antlr4#build"]
-    },
-    "@neo4j-cypher/language-support#build": {
-      "dependsOn": ["antlr4#build", "antlr4-c3#build"]
-    },


I don't think removing these is correct?

Now when I do npm run clean --force and npm run build it cannot find antlr4 or antlr4-c3:

antlr4:build: cache miss, executing 2607cb7a1baba40d @neo4j-cypher/language-support:build: cache miss, executing 1d715a04baa6bc36 @neo4j-cypher/language-support:build: @neo4j-cypher/language-support:build: > @neo4j-cypher/language-support@2.0.0-next.3 build @neo4j-cypher/language-support:build: > npm run gen-parser && concurrently 'npm:build-types' 'npm:build-esm' 'npm:build-commonjs' @neo4j-cypher/language-support:build: antlr4:build: antlr4:build: > antlr4@4.13.1 build antlr4:build: > webpack antlr4:build: @neo4j-cypher/language-support:build: @neo4j-cypher/language-support:build: > @neo4j-cypher/language-support@2.0.0-next.3 gen-parser @neo4j-cypher/language-support:build: > antlr4 -Dlanguage=TypeScript -visitor src/antlr-grammar/CypherCmdLexer.g4 src/antlr-grammar/CypherCmdParser.g4 -o src/generated-parser/ -Xexact-output-dir @neo4j-cypher/language-support:build: @neo4j-cypher/language-support:build: warning(109): CypherParser.g4:20:0: options ignored in imported grammar CypherParser @neo4j-cypher/language-support:build: [build-types] @neo4j-cypher/language-support:build: [build-types] > @neo4j-cypher/language-support@2.0.0-next.3 build-types @neo4j-cypher/language-support:build: [build-types] > tsc --emitDeclarationOnly --outDir dist/types @neo4j-cypher/language-support:build: [build-types] @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] > @neo4j-cypher/language-support@2.0.0-next.3 build-esm @neo4j-cypher/language-support:build: [build-esm] > esbuild ./src/index.ts --bundle --format=esm --sourcemap --outfile=dist/esm/index.mjs @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-commonjs] @neo4j-cypher/language-support:build: [build-commonjs] > @neo4j-cypher/language-support@2.0.0-next.3 build-commonjs @neo4j-cypher/language-support:build: [build-commonjs] > esbuild ./src/index.ts --bundle --format=cjs --sourcemap --outfile=dist/cjs/index.cjs @neo4j-cypher/language-support:build: [build-commonjs] @neo4j-cypher/language-support:build: [build-esm] ✘ [ERROR] Could not resolve "antlr4" @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] src/helpers.ts:8:7: @neo4j-cypher/language-support:build: [build-esm] 8 │ } from 'antlr4'; @neo4j-cypher/language-support:build: [build-esm] ╵ ~~~~~~~~ @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] The module "./dist/antlr4.web.mjs" was not found on the file system: @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] ../../node_modules/antlr4/package.json:64:18: @neo4j-cypher/language-support:build: [build-esm] 64 │ "import": "./dist/antlr4.web.mjs", @neo4j-cypher/language-support:build: [build-esm] ╵ ~~~~~~~~~~~~~~~~~~~~~~~ @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] You can mark the path "antlr4" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle. @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-commonjs] ✘ [ERROR] Could not resolve "antlr4" @neo4j-cypher/language-support:build: [build-commonjs] @neo4j-cypher/language-support:build: [build-commonjs] src/signatureHelp.ts:6:32: @neo4j-cypher/language-support:build: [build-commonjs] 6 │ import { ParseTreeWalker } from 'antlr4'; @neo4j-cypher/language-support:build: [build-commonjs] ╵ ~~~~~~~~ @neo4j-cypher/language-support:build: [build-commonjs] @neo4j-cypher/language-support:build: [build-commonjs] The module "./dist/antlr4.web.mjs" was not found on the file system: @neo4j-cypher/language-support:build: [build-commonjs] @neo4j-cypher/language-support:build: [build-commonjs] ../../node_modules/antlr4/package.json:64:18: @neo4j-cypher/language-support:build: [build-commonjs] 64 │ "import": "./dist/antlr4.web.mjs", @neo4j-cypher/language-support:build: [build-commonjs] ╵ ~~~~~~~~~~~~~~~~~~~~~~~ @neo4j-cypher/language-support:build: [build-commonjs] @neo4j-cypher/language-support:build: [build-commonjs] You can mark the path "antlr4" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle. @neo4j-cypher/language-support:build: [build-commonjs] @neo4j-cypher/language-support:build: [build-esm] ✘ [ERROR] Could not resolve "antlr4" @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] src/signatureHelp.ts:6:32: @neo4j-cypher/language-support:build: [build-esm] 6 │ import { ParseTreeWalker } from 'antlr4'; @neo4j-cypher/language-support:build: [build-esm] ╵ ~~~~~~~~ @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] The module "./dist/antlr4.web.mjs" was not found on the file system: @neo4j-cypher/language-support:build: [build-esm] @neo4j-cypher/language-support:build: [build-esm] ../../node_modules/antlr4/package.json:64:18: @neo4j-cypher/language-support:build: [build-esm] 64 │ "import": "./dist/antlr4.web.mjs", @neo4j-cypher/language-support:build: [build-esm] ╵ ~~~~~~~~~~~~~~~~~~~~~~~

You can also see it in the CI failed unit tests job

Ah it seems you're right for our vendored deps, wonder why the automatic "build dependencies first" doesn't work for those.. Anyway, I left in the manual spec for those two in now 👍

Hopefully we shouldn't need to use clean or --force 🙏

ncordon · 2024-04-02T17:41:42Z

turbo.json

-      "persistent": false,
-      "dependsOn": ["@neo4j-cypher/react-codemirror#test:e2e"]
+      "outputs": [],
+      "dependsOn": ["^build"],


Is this correct? From the docs of Turbo, they say this makes build act on the dependencies before running build? Why do we need that?

// ^build means `build` must be run in dependencies // before it can be run in this workspace

When I open the codemirror debug and I change something in language-support, I'd expect the build to reload, but it doesn't? 🙈

This means that the build scripts in the dependencies must run before the dev script can run, since we'd need @neo4j-cypher/react-codemirror to be built. As I read the docs, the ^scriptName syntax is "run scriptName" in in dependencies first and the example happens to be build + build

Second: Turbo doesn't support watching for file changes.. I mentioned this in the PR description as one of the main reasons to switch to wireit or some other simpler&better tool

ncordon · 2024-04-02T18:27:04Z

turbo.json

@@ -2,8 +2,16 @@
  "$schema": "https://turbo.build/schema.json",
  "pipeline": {
    "build": {
-      "outputs": ["out/**", "dist/**"],
-      "dependsOn": ["^build"]
+      "outputs": ["dist/**"],


Do we need to clean a Turbo local cache for this to work?

It's still telling me cache hit for antlr4-c3 but the dist folder will not show up inside vendor/antlr4-c3/dist.

Maybe erasing node_modules/.cache/turbo does the trick?

Could you tryrm -rf node_modules and then run the build scripts without using --force?

ncordon · 2024-04-03T13:47:06Z

turbo.json

+      "dependsOn": ["antlr4#build"],
+      "outputs": ["dist/**"]


I think we need this specified to cache dist, given we had to declare antlr4-c3#build explicitly. Otherwise if you were deleting antlr4-c3/dist and rerunning with npm run build, we would not see that output recreated.

ncordon · 2024-04-03T13:53:54Z

packages/language-server/package.json

@@ -35,6 +35,7 @@
  },
  "dependencies": {
    "@neo4j-cypher/language-support": "2.0.0-next.5",
+    "@neo4j-cypher/schema-poller": "2.0.0-next.5",


This is not going to work @OskarDamkjaer. Basically there was a reason why these dependencies were listed in the turbo.json instead. If you declare this here and the package schema-poller is not published to npm, you will not be able to use the distributed language server from there.

ncordon

I'm blocking this because I realize this would break the packages in npm.

schema-poller cannot be a explicit dependency of vscode-extension because we don't deliver that on npm, we just bundle it.

fixes clean/build cache

ab85abc

OskarDamkjaer marked this pull request as draft March 1, 2024 13:59

OskarDamkjaer added 3 commits March 1, 2024 15:12

simplify dev cmd

5164b28

mergemain

efc1281

add pre launch task

289b26d

OskarDamkjaer changed the title ~~fixes clean/build cache~~ fixes clean/build cache & simplify vscode debugging Mar 1, 2024

OskarDamkjaer assigned ncordon Mar 1, 2024

OskarDamkjaer marked this pull request as ready for review March 1, 2024 14:30

OskarDamkjaer requested a review from ncordon March 1, 2024 14:31

ncordon reviewed Mar 18, 2024

View reviewed changes

OskarDamkjaer added 2 commits April 2, 2024 09:18

merge main

3d0d50f

adds one manual dep

21066d5

OskarDamkjaer requested a review from ncordon April 2, 2024 07:39

ncordon reviewed Apr 2, 2024

View reviewed changes

ncordon added 2 commits April 3, 2024 14:42

Fixes caching output of antlr4-c3

dde3665

Adds watch task that works in codemirror

ad91f5c

ncordon reviewed Apr 3, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into turbo-fixes

4c1191a

ncordon reviewed Apr 3, 2024

View reviewed changes

ncordon requested changes Apr 3, 2024

View reviewed changes

Reverts part of the turbo dependency graph

151c3aa

ncordon changed the title ~~fixes clean/build cache & simplify vscode debugging~~ fixes clean/build cache & brings back files watch Apr 3, 2024

ncordon approved these changes Apr 4, 2024

View reviewed changes

ncordon merged commit 8b0052c into main Apr 4, 2024
4 checks passed

ncordon deleted the turbo-fixes branch April 4, 2024 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes clean/build cache & brings back files watch #183

fixes clean/build cache & brings back files watch #183

OskarDamkjaer commented Mar 1, 2024 •

edited

Loading

ncordon Mar 18, 2024 •

edited

Loading

ncordon Mar 18, 2024

OskarDamkjaer Apr 2, 2024

OskarDamkjaer Apr 2, 2024

ncordon Apr 2, 2024

ncordon Apr 2, 2024

OskarDamkjaer Apr 3, 2024

ncordon Apr 2, 2024 •

edited

Loading

OskarDamkjaer Apr 3, 2024

ncordon Apr 3, 2024 •

edited

Loading

OskarDamkjaer Apr 4, 2024

ncordon Apr 3, 2024 •

edited

Loading

ncordon left a comment •

edited

Loading

fixes clean/build cache & brings back files watch #183

fixes clean/build cache & brings back files watch #183

Conversation

OskarDamkjaer commented Mar 1, 2024 • edited Loading

ncordon Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncordon Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncordon Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncordon Apr 3, 2024 • edited Loading

Choose a reason for hiding this comment

ncordon left a comment • edited Loading

Choose a reason for hiding this comment

OskarDamkjaer commented Mar 1, 2024 •

edited

Loading

ncordon Mar 18, 2024 •

edited

Loading

ncordon Apr 2, 2024 •

edited

Loading

ncordon Apr 3, 2024 •

edited

Loading

ncordon Apr 3, 2024 •

edited

Loading

ncordon left a comment •

edited

Loading