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

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

Merged
merged 5 commits into from
Dec 9, 2023

Conversation

markw65
Copy link

@markw65 markw65 commented Sep 8, 2023

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
Copy link
Author

markw65 commented Sep 8, 2023

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

package.json Outdated
Comment on lines 89 to 90
"compilerOptions": {
"types": ["jest"]
Copy link
Author

Choose a reason for hiding this comment

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

There's no way (that I can find) to tell tsd to use anything other than tsconfig.json, so we have to override the types field here.

It might have been cleaner to keep tsconfig.json as the "build" config, and use tsconfig-lint.json for vscode's typescript validation. But vscode doesn't support using a different file name either.

@hildjj
Copy link
Contributor

hildjj commented Dec 7, 2023

This one next, please. Rebase and changelog.

@markw65
Copy link
Author

markw65 commented Dec 8, 2023

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
Copy link
Author

markw65 commented Dec 8, 2023

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.

@hildjj hildjj merged commit 55e8b6e into peggyjs:main Dec 9, 2023
9 checks passed
@markw65 markw65 mentioned this pull request Dec 9, 2023
@markw65 markw65 deleted the tsconfig branch December 10, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tsconfig targets es5, but allows es2020 functions
2 participants