fix: typescript-app template broken for npm > 7 #362
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: iliapolo <epolon@amazon.com>
iliapolo
commented
Jun 28, 2022
| name: 'cdk8s-cli', | ||
| description: 'This is the command line tool for Cloud Development Kit (CDK) for Kubernetes (cdk8s).', | ||
| repositoryUrl: 'https://github.com/cdk8s-team/cdk8s-cli.git', | ||
| projenUpgradeSecret: 'PROJEN_GITHUB_TOKEN', | ||
| authorName: 'Amazon Web Services', | ||
| authorUrl: 'https://aws.amazon.com', | ||
| minNodeVersion: '14.17.0', | ||
| defaultReleaseBranch: 'main', |
There was a problem hiding this comment.
Leftover from before we had a branch per major version
iliapolo
commented
Jun 28, 2022
| @@ -62,11 +63,12 @@ const project = new typescript.TypeScriptProject({ | |||
| 'typescript-json-schema', | |||
| ], | |||
|
|
|||
| // we need the compiled .js files for the init tests (we run the cli in there) | |||
| compileBeforeTest: true, | |||
There was a problem hiding this comment.
Not necessary anymore since init templates are not part of the test command anymore
Signed-off-by: iliapolo <epolon@amazon.com>
iliapolo
changed the title
RomainMuller
approved these changes
Jun 29, 2022
⚪ Backport skippedThe pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually. Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
iliapolo
added a commit
that referenced
this pull request
Jun 29, 2022
(cherry picked from commit 77bc7b3) Signed-off-by: Eli Polonsky <epolon@amazon.com>
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
mergify bot
pushed a commit
that referenced
this pull request
Jul 6, 2022
# Backport This will backport the following commits from `2.x` to `1.x`: - [fix: typescript-app template broken for npm > 7 (#362)](#362) ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)
RomainMuller
added a commit
that referenced
this pull request
Jul 27, 2022
#362 made the `compile` and `watch` scripts of the `typescript-app` template use a relative path to the `tsc` compiler, however such a form does not work as intended on Windows... Instead of doing this, explicitly declare a `devDependency` on the `typescript` compiler and return to using just `tsc`. Also, pass the `--build` argument, which speeds up incremental builds considerably.
RomainMuller
added a commit
that referenced
this pull request
Jul 27, 2022
#362 made the `compile` and `watch` scripts of the `typescript-app` template use a relative path to the `tsc` compiler, however such a form does not work as intended on Windows... Instead of doing this, explicitly declare a `devDependency` on the `typescript` compiler and return to using just `tsc`. Also, pass the `--build` argument, which speeds up incremental builds considerably. Signed-off-by: 🧑🏻💻 Romain Marcadier <rmuller@amazon.com>
mergify bot
pushed a commit
that referenced
this pull request
Aug 28, 2022
#362 made the `compile` and `watch` scripts of the `typescript-app` template use a relative path to the `tsc` compiler, however such a form does not work as intended on Windows... Instead of doing this, explicitly declare a `devDependency` on the `typescript` compiler and return to using just `tsc`. Also, pass the `--build` argument, which speeds up incremental builds considerably.
cdk8s-automation
pushed a commit
that referenced
this pull request
Aug 28, 2022
#362 made the `compile` and `watch` scripts of the `typescript-app` template use a relative path to the `tsc` compiler, however such a form does not work as intended on Windows... Instead of doing this, explicitly declare a `devDependency` on the `typescript` compiler and return to using just `tsc`. Also, pass the `--build` argument, which speeds up incremental builds considerably. (cherry picked from commit 43d9db1) Signed-off-by: Romain Marcadier <rmuller@amazon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
typescript-apptemplate has a few npm scripts in itspackage.jsonthat runtsc.By default, the
tsccommand resolves to./node_modules/.bin/tsc. This used to be ok, until a change made it so this bin script gets overridden by a different typescript version than the one declared in the template. A fix has already been merged, which should take of this issue. However, we can also fix it in our template by having the scripts point to the expectedtscpath.When investigating this issue, it also struck me as weird this wasn't detected in the cli init templates tests.
This is because up until now, our init template were using the source code of the CLI to run against, instead of the tarball.
In addition, we did not have any tests executed against newer versions of node, where npm got a major version bump and underwent some substantial behavior changes.
So, this PR also adds the following:
Also, switch to typescript for projen file.
Fixes #363