Skip to content

Enable requiring the package.json #33

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

Merged
merged 1 commit into from
Nov 3, 2021
Merged

Enable requiring the package.json #33

merged 1 commit into from
Nov 3, 2021

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Nov 3, 2021

Description

The package.json is a useful to be able to read inside the source code. This is because it contains version information that we often need to provide in our own program.

There are 2 ways of acquiring the package.json:

import packageJson from '../package.json';
const packageJson = require('../package.json');

The former is using ES6 imports, and will trigger typescript to resolve and compile it.

TSC will ask for resolveJsonModule: true option in tsconfig.json. However if we enable it to be true, then the package.json file will be copied in the dist/ directory.

dist
├── package.json
└── src

At this point in time, we don't really want this, although we could adapt by changing the main and types path in package.json.

Ideally TSC will just not copy package.json, but there's only 2 ways to do this.

First way is to remove resolveJsonModule: true so that it's not resolved. Then make tsc ignore the error:

// @ts-expect-error package.json is outside the compiled dist
import { version } from '../package.json';

Then when compiling it works.

Second way, just use require, and tsc will ignore it.

The second way is not as good idea if we want to later move to ES6 style imports/exports as in #32.

The first way has one limitation, the resolveJsonModule must be false. This means we are making TSC not check/resolve the json module at all and therefore there's no type information. This means we cannot use import json module anywhere, and TSC won't have any type information about JSON that is imported.

This can be a problem, and then JSON would have to be considered non-source code, but instead static assets that should be acquired elsewhere.

An alternative to this solution is https://stackoverflow.com/a/61467483/582917 and https://stackoverflow.com/a/61467483/582917. These suggest creating a tsconfig.json under src as well as tsconfig.json in the root.

In our PK code, we have to use resolveJsonModule: true (due to our json schema), so our only option is to use require right now. We must continue this PR to try the other solutions so that #32 can be used.

Tasks

  1. - Try out the require method
  2. - Try out @ts-ignore method
  3. - Try out the super and subproject method

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member Author

A dynamic import has the same problem, if --resolveJsonModule is used it ends up acquiring it and copying it. But in that case no need for @ts-expect-error.

@CMCDragonkai
Copy link
Member Author

Dynamic imports would require top level await. And that requires using ES6 modules. So for now that's not an option until we try #32.

But I found a solution to this problem.

The idea is to use rootDir to say ./src. This forces TSC to always treat src as the root dir. Then when importing from ../package.json it will cause an error. This is when we use @ts-ignore to ignore it.

Strangely, the @ts-expect-error cannot be used, I guess it's in a different part of the pipeline. But doing this allows us to enable resolveJsonModule and then we can in fact import JSON from inside ./src and ../package.json is just a special case.

@CMCDragonkai
Copy link
Member Author

Ok I think this will work in PK, merging.

@CMCDragonkai CMCDragonkai merged commit b9e5385 into master Nov 3, 2021
@CMCDragonkai CMCDragonkai deleted the packagejson branch November 3, 2021 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant