Skip to content

Move typescript to a peer dependency #110

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.

Sign up for GitHub

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 7 commits into from
Feb 2, 2022
Merged

Move typescript to a peer dependency #110

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
de6ded9
Move typescript to a peer dependency
wagenet Jan 20, 2022
3134817
Test multiple Typescript versions
wagenet Jan 24, 2022
81edec3
Update minimum Node version
wagenet Jan 28, 2022
15edc37
Separate expectations by TS version
wagenet Feb 1, 2022
94972fb
Don't cache submodule
wagenet Feb 1, 2022
bccc957
Improve CI config
wagenet Feb 1, 2022
ba7b625
Bump version
wagenet Feb 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,22 @@ on:

jobs:
test:
name: "Node ${{ matrix.node }} - ${{ matrix.os }}"
name: "Node ${{ matrix.node }} / TS: ${{ matrix.typescript }} - ${{ matrix.os }}"
runs-on: "${{ matrix.os }}-latest"

strategy:
fail-fast: false
matrix:
node: ['10', '12', '14']
os: ['windows', 'ubuntu']
typescript: ['default']
include:
- node: 10
os: ubuntu
typescript: ~4.0.3
- node: 14
os: ubuntu
typescript: latest

steps:
- name: Ensure long path support
Expand All @@ -28,7 +37,26 @@ jobs:
- uses: volta-cli/action@v1
with:
node-version: ${{ matrix.node }}
- name: Cache
uses: actions/cache@v2
with:
path: |
node-modules
vendor/typescript
key: ${{ runner.os }}-${{ matrix.node }}-${{ hashFiles('**/package-lock.json') }}-${{ matrix.typescript }}
restore-keys: |
${{ runner.os }}-${{ matrix.node }}-${{ hashFiles('**/package-lock.json') }}-${{ matrix.typescript }}
${{ runner.os }}-${{ matrix.node }}-${{ hashFiles('**/package-lock.json') }}
${{ runner.os }}-${{ matrix.node }}-
- name: install dependencies
run: yarn --frozen-lockfile
- run: git submodule update --init
- name: Switch to alternate TS
if: matrix.typescript != 'default'
run: |
yarn upgrade typescript@${{ matrix.typescript }}
version=$(yarn tsc -v | grep Version | cut -d ' ' -f 2 | cut -d '.' -f 1,2)
cd vendor/typescript
git checkout release-${version}
- run: yarn lint
- run: yarn test
- run: yarn test:no-update
10 changes: 7 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
"link-self": "yarn link && yarn link broccoli-typescript-compiler",
"lint": "tslint --project tsconfig.json && tslint --project tests/tsconfig.json",
"prepare": "npm run build",
"test": "git submodule update --init && qunit dist/tests/*-test.js"
"test": "git submodule update --init && yarn test:no-update",
"test:no-update": "qunit dist/tests/*-test.js"
},
"dependencies": {
"broccoli-funnel": "^3.0.3",
Expand All @@ -45,7 +46,6 @@
"fs-tree-diff": "^2.0.1",
"heimdalljs": "0.3.3",
"md5-hex": "^3.0.1",
"typescript": "~4.0.3",
"walk-sync": "^2.1.0"
},
"devDependencies": {
Expand All @@ -65,7 +65,11 @@
"rollup-plugin-sourcemaps": "^0.5.0",
"tslint": "^5.11.0",
"tslint-config-prettier": "^1.13.0",
"tslint-plugin-prettier": "^1.3.0"
"tslint-plugin-prettier": "^1.3.0",
"typescript": "^4.5.5"
},
"peerDependencies": {
"typescript": "^4.0.3"
Copy link

@SergeAstapov SergeAstapov Jan 28, 2022

Choose a reason for hiding this comment

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

@wagenet I may be wrong, but this not gonna make any difference as still will install typescript@4.0.3 by npm 7+

It's technically the same as replacing "typescript": "~4.0.3" with "typescript": "^4.0.3".

To truly decouple typescript version from broccoli-typescript-compiler, maybe you meant to make this

"peerDependencies": {
    "typescript": "*"
},
"peerDependenciesMeta": {
  "typescript": {
    "optional": true
  }
},

Copy link
Collaborator Author

@wagenet wagenet Jan 28, 2022

Choose a reason for hiding this comment

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

@SergeAstapov ah, you may be correct.

Copy link
Collaborator Author

@wagenet wagenet Jan 28, 2022

Choose a reason for hiding this comment

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

My understanding is that this means that, if a newer compatible version is installed, it will use that one. So this should be correct. I've been having some trouble validating it in practice either way.

},
"engine": "node >= 10.0.0",
"volta": {
Expand Down
12 changes: 11 additions & 1 deletion src/fs/file-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@ import {
Resolution,
} from "../interfaces";

interface CodedError extends Error {
code: string;
}

function isCodedError(value: unknown): value is CodedError {
return (
value instanceof Error && typeof (value as CodedError).code === "string"
);
}

export function readFile(path: AbsolutePath): FileContent {
const buffer = readFileSync(path);
const hash = createHash("sha1");
Expand All @@ -34,7 +44,7 @@ export function stat(path: AbsolutePath): Stats | undefined {
try {
return statSync(path);
} catch (e) {
if (e.code === "ENOENT" || e.code === "EACCES") {
if (isCodedError(e) && (e.code === "ENOENT" || e.code === "EACCES")) {
return;
}
throw e;
Expand Down
12 changes: 10 additions & 2 deletions tests/throw-on-error-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ QUnit.module("throwOnError", function({ beforeEach, afterEach }) {
try {
await output.build();
} catch (e) {
assertMatches(assert, e.message, /TS2322/);
assertMatches(
assert,
e instanceof Error ? e.message : String(e),
/TS2322/
);
}
}
);
Expand All @@ -99,7 +103,11 @@ QUnit.module("throwOnError", function({ beforeEach, afterEach }) {
try {
await output.build();
} catch (e) {
assertMatches(assert, e.message, /TS2322/);
assertMatches(
assert,
e instanceof Error ? e.message : String(e),
/TS2322/
);
}
}
);
Expand Down
2 changes: 1 addition & 1 deletion vendor/typescript
Open in desktop
Submodule typescript updated 10482 files
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2694,10 +2694,10 @@ type@^2.0.0:
resolved "https://registry.yarnpkg.com/type/-/type-2.0.0.tgz#5f16ff6ef2eb44f260494dae271033b29c09a9c3"
integrity sha512-KBt58xCHry4Cejnc2ISQAF7QY+ORngsWfxezO68+12hKV6lQY8P/psIkcbjeHWn7MqcgciWJyCCevFMJdIXpow==

typescript@~4.0.3:
version "4.0.3"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.0.3.tgz#153bbd468ef07725c1df9c77e8b453f8d36abba5"
integrity sha512-tEu6DGxGgRJPb/mVPIZ48e69xCn2yRmCgYmDugAVwmJ6o+0u1RI18eO7E7WBTLYLaEVVOhwQmcdhQHweux/WPg==
typescript@^4.5.5:
version "4.5.5"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.5.5.tgz#d8c953832d28924a9e3d37c73d729c846c5896f3"
integrity sha512-TCTIul70LyWe6IJWT8QSYeA54WQe8EjQFU4wY52Fasj5UKx88LNYKCgBEHcOMOrFF1rKGbD8v/xcNWVUq9SymA==

uglify-js@^3.1.4:
version "3.8.0"
Expand Down