peggyjs / peggy

Peggy: Parser generator for JavaScript
https://peggyjs.org/
MIT License
906 stars 64 forks source link

Setup tsconfig to detect use of library functions from es6 or later #435

Closed markw65 closed 8 months ago

markw65 commented 11 months ago

Fixes #431

I'm not 100% sure that detecting uses of es6 functions is necessary. At the least, String#endsWith is already in use in generate-js.js, and Array.from is in use in stack.js.

But right now anything through es2020 is accepted, and that's probably not intentional. So I think this probably needs to be updated to es2015 (or es6); or something needs to be done with rollup to provide pollyfills.

I discovered that the fix to get ts to respect the "target" spec, even in the presence of @types/node (and others) is to set "types":[]. The problem with that is that it causes tsc to complain that describe and it aren't defined (currently not a problem, because none of the tests specify @ts-check). That can be fixed by setting "types":["jest"], but /that/ has the side effect of pulling in @types/node again.

It turns out that tslint can handle this nicely: just put a tsconfig.json in the test directory which sets "types":["jest"]. This ensures that vscode shows problems correctly for both. But tsc --build doesn't look at tsconfig.json files in subfolders, so the build doesn't work.

So to fix that, I've added a tsconfig-build.json file that's equivalent to the original tsconfig.json: it will build everything, but won't warn about newer library functions. But tslint only reports on files that are open in vscode; and we don't want to rely on people using vscode anyway. So I've modified npm run ts to run both tsconfig-build.json, and tsconfig.json to get any errors. Since tsconfig.json is now only used for linting, I've also changed it to be noEmit by default.

Finally, since we now have 3 copies of tsconfig, with only small differences, I created a tsconfig-base.json, and had the others extend from it appropriately.

There may be a simpler solution - but I couldn't find it.

markw65 commented 11 months ago

Sorry, need to fix peg.test-d.ts

hildjj commented 8 months ago

This one next, please. Rebase and changelog.

markw65 commented 8 months ago

Note - this isn't just a rebase; this is a complete rewrite. I realized that the previous version wasn't working the way I expected. In particular, the build still didn't warn about the use of non-es5 functions, even though vscode did (provided you had the file open).

But I think this is a much better approach. I switched over to using typescript project references. These allow one tsconfig to reference another, and cause tsc --build to follow the references, resolve them, and build each sub-project with its own set of options.

One downside is that eslint doesn't understand them; but that can be fixed by giving it a list of the subprojects.

Anyway, it now works as intended. And as expected reported that String.endsWith() is not a function. To fix that I updated the lib to ES2015. I assume that's ok, because String.endsWith() has been in the release for a long time.

I decided not to update target to ES2015 because doing so dramatically changes the output - let and const instead of var for one thing - and I'm not sure how universal support for ES2015 syntax is.

markw65 commented 8 months ago

Also, I realized that in the previous couple of pull requests, I had forgotten to update the build artifacts. Most of the build artifact changes in this pull request are from previous pull requests - there's one small change to the test bundle because it includes package.json, and that the ts npm script changed.