Use project references to fix build #335
Conversation
* Fix include paths for extension * Correct usage of rootDir and baseUrl * Remove superfluous tsconfig from SmoldotProvider directory * Fix ConnectionManager imports in burnr * Use `tsc -b` everywhere to get incremental builds
| "clean": "rm -rf dist", | ||
| "dev": "yarn run clean && tsc --noEmit & parcel ./public/index.html", | ||
| "clean": "rm -rf dist tsconfig.tsbuildinfo", | ||
| "dev": "yarn run clean && tsc -b & parcel ./public/index.html", |
There was a problem hiding this comment.
Why remove the noEmit? tsc is typechecking while parcel takes care of compiling the code. Afaiu this change will typecheck, build and then parcel will compile the code to show it for dev.
There was a problem hiding this comment.
You can't use -b and --noEmit together. I decided it would be preferable to have incremental building and detection of changes in dependencies than suppressing the build output
There was a problem hiding this comment.
Here is the justification from the typescript team on why noEmit wont build dependent references with --noEmit - microsoft/TypeScript#40431
There was a problem hiding this comment.
I agree with the above. noEmit is used for not emitting compiler output files like JavaScript source code, source-maps or declarations based on doc: https://www.typescriptlang.org/tsconfig#noEmit allowing other tools to do the compiling. All I am saying is that so far we were using parcel to compile our project while keep tsc (with --noEmit) just for typechecking. By replacing --noEmit with -b we now use tsc to typecheck and compile, and then parcel to compile again.
There was a problem hiding this comment.
The point is a user cannot just clone then cd into burnr and run yarn dev if we use --noEmit ... I think that is surprising. By using -b instead of --noEmit its true that the compile is slightly slower than using --noEmit but its "bulletproof" .. it will always do what you expect no matter the state of the other projects (it will compile them if needed)
There was a problem hiding this comment.
Also if you make changes to @substrate/connect (and don't recompile it) then cd into one of the demos and then run yarn dev in the demo you will not have the changes in @substrate/connect since typescript with --noEmit won't check the dependency for changes - it will use the stale build output from @substrate/connect. That's also surprising
There was a problem hiding this comment.
Regarding the slowdown by doing a full compile instead of using --noEmit.. that is probably only a concern for the first hit. -b does incremental compilation so after the first build it will have build caches and so is able to optimise the build time depending on what has changed
There was a problem hiding this comment.
I'm not sure I follow. I just replicated the steps you mentioned (after changing the -b to --noEmit ofc) and I was able to yarn dev from inside burnr dir.
| "clean": "rm -rf dist", | ||
| "dev": "yarn run clean && tsc --noEmit & parcel ./public/index.html", | ||
| "clean": "rm -rf dist tsconfig.tsbuildinfo", | ||
| "dev": "yarn run clean && tsc -b & parcel ./public/index.html", |
There was a problem hiding this comment.
by removing -noEmit then "tsc" command also compiles the code instead of just typechecking. We are using parcel for that
There was a problem hiding this comment.
See above .. we can't detect changes in dependent projects (and therefore typecheck properly) with noEmit
| "typecheck": "tsc --noEmit", | ||
| "typecheck:watch": "tsc --noEmit --watch", | ||
| "clean": "rm -rf dist/ tsconfig.tsbuildinfo && mkdir -p dist/assets", | ||
| "typecheck": "tsc -b", |
There was a problem hiding this comment.
by removing -noEmit then "typecheck" command also compiles the code instead of just typechecking. We are using parcel for that
tsc -beverywhere to get incremental buildsThis fixes #292