fix(npm-scripts): bundle TypeScript dependencies #114
Merged
Conversation
This file contains hidden or 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
|
Marking this one as a draft because it depends on #113 going in first. |
|
I'm amazed that GitHub actually did something useful for once. As soon as I:
...it did this:
Which is exactly what I wanted it to do... (not sure if it did it it after (1) or (2) or (3), but whatever).
|
As reported here:
#91 (comment)
Our use of the `@typescript-eslint/parser` (since 50a4d99) means
that we have a runtime dependency on `typescript` package. This worked
fine in liferay-portal, where `typescript` is already present, but not
in liferay-learn, where it is not. And I didn't know that
`@typescript-eslint/parser` doesn't actually declare all of its
transitive dependencies; as noted in:
#113
`@typescript-eslint/typescript-estree` does an unguarded `import` of
`typescript` even though it is not declared in its dependencies (they
declare it as an optional peer dependency).
So, let's not only fix this, by explicitly declaring our dependency on
TypeScript, but also declare all of our TS-related dependencies. This
will allow us to delete the devDependencies currently declared here:
https://github.com/liferay/liferay-portal/blob/f1a2b2ce06fa050d5e1ab193728ab9ec06df4315/modules/apps/remote-app/remote-app-client-js/package.json#L3-L6
wincent
force-pushed
the
wincent/ts-first-class-citizen-ii
branch
from
21b8e40 to
22765d7
Compare
With `npx yarn-deduplicate yarn.lock`.
|
As this one is intimately related to the (merged/approved #113) I'm going to merge this one too. |
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.
As reported here, our use of the
@typescript-eslint/parser(since 50a4d99) means that we have a runtime dependency ontypescriptpackage. This worked fine in liferay-portal, wheretypescriptis already present, but not in liferay-learn, where it is not. And I didn't know that@typescript-eslint/parserdoesn't actually declare all of its transitive dependencies; as noted in #113,@typescript-eslint/typescript-estreedoes an unguardedimportoftypescripteven though it is not declared in its dependencies (they declare it as an optional peer dependency).So, let's not only fix this, by explicitly declaring our dependency on TypeScript, but also declare all of our TS-related dependencies. This will allow us to delete the devDependencies currently declared here.