ESLint Config Migration: Use tsconfig's resolveJsonModule, replace require with import #1053
Conversation
filmaj
force-pushed
the
resolve-json-module
branch
from
e44786c to
c92a332
Compare
There was a problem hiding this comment.
I actually think this breaks the distribution. I made this change last year, when it comes time to release, the tar file generated is broken because pakage.json lives outside of src. So previously, when generating the tar for release (npm pack), src is the root directory. With this change, the root changes to package.json and src becomes a sibling of that.
|
@stevengill Great point! I've confirmed that |
filmaj
force-pushed
the
resolve-json-module
branch
from
c92a332 to
6b2203e
Compare
|
@stevengill understood, but I wonder if that is a separate packaging/distribution problem? Indeed, should we adopt this particular PR, the releasing process would need to be updated, but perhaps these are two separate problems that should be examined independently? Assuming I understand correctly, could we solve the issue by updating / automating the release process with a script that moves any files around as needed? Related: can anyone share details around how packaging/releasing/distribution is done? The maintainer's guide is a bit light on the specific steps. When I run 5 files, no code! 🙈 so, I am clearly doing something wrong! |
filmaj
force-pushed
the
resolve-json-module
branch
from
6b2203e to
e03441f
Compare
|
I think I see the problem... the project structure seems to be confounding changes between different PRs, causing the structure of the final package to be muddled. I have added a new commit that ensures a The directory structure now looks a bit better, with only a single I can also extract the tar file, WDYT @stevengill @seratch ? |
…mport for importing JSON
filmaj
force-pushed
the
resolve-json-module
branch
from
e03441f to
c3f46b1
Compare
|
@filmaj |
|
It sounds like using I have so far tried two different approaches:
I will continue to investigate and see what further options exist / can be used. |
…Project References to link them all up. Provide a dummy package.json and d.ts file for the types-tests to get them running. Be more specific about which files to include in packaging so as to avoid including the tsconfig.tsbuildinfo file.
|
OK! Well, I have some kind of solution, though I think it is hardly "nice". The core of the solution relies on Typescript Project References. This effectively splits up bolt-js into "subprojects" from TypeScript's perspective. As such, there is not one, but now three!
In order to get the I can likely severely cut down on the number of lines making up So, given all the above, what do people think? Is the amount of configuration, dummy files, etc. worth it? The other option is to leave the old way of importing Personally, having put half a day into trying to get |
| @@ -18,20 +18,22 @@ | |||
| "main": "./dist/index.js", | |||
| "types": "./dist/index.d.ts", | |||
| "files": [ | |||
| "dist/**/*" | |||
| "dist/**/*.map", | |||
There was a problem hiding this comment.
This change is to avoid including tsconfig.tsbuildinfo, which is a new "cache" file tsc uses when you use Project References and the tsc --build command.
| ], | ||
| "engines": { | ||
| "node": ">=12.13.0", | ||
| "npm": ">=6.12.0" | ||
| }, | ||
| "scripts": { | ||
| "prepare": "npm run build", | ||
| "build": "tsc", | ||
| "build": "npm run build:clean && tsc --build src", |
There was a problem hiding this comment.
More information on tsc --build available in the Project References TypeScript documentation.
| @@ -81,8 +83,5 @@ | |||
| "ts-node": "^8.1.0", | |||
| "tsd": "^0.13.1", | |||
| "typescript": "^4.1.0" | |||
| }, | |||
| "tsd": { | |||
There was a problem hiding this comment.
I removed this section in favour of explicitly specifying the directory to run tsd in via the scripts' test:types run script section above. IMO it is a bit clearer - as a new user of tsd myself, I did not know that tsd would look for a tsd section in package.json to look for invocation options.
|
@filmaj Thanks for jumping into the deep world of the tsconfig 😺
I don't think that it's worth it.
Yes, your understanding here is correct. As the |
|
Closing this pull request in favour of keeping the ol' |
Summary
This is a PR that should be merged into #1024 and incrementally addresses #842.
This PR sets
resolveJsonModulesto true in thetsconfig.json, allowing for use ofimporton .json files, and thus fixing the lint failures around use ofrequire.Impact
Before
After
Requirements (place an
xin each[ ])