feat: add support for formatting and linting TypeScript #97
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
Note that the goal here is to format and lint TypeScript in liferay-portal, but we're "dog-fooding" in this monorepo already, even though there isn't any TypeScript in here (yet... when we bring across the js-toolkit packages TypeScript will start coming in here too). Note how we can lint JS just fine with the TS parser. Closes: #96
|
This one is a draft because we very, very likely are going to need some more tweaks when we come to actually try integrating it. My initial smoke test over in the liferay-portal shows some things exploding in interesting ways 😂. cc @julien |
|
This is an attempt to address some of the problems described in this comment: #97 (comment) Namely, that some of our JS files have namespaced JSX attributes in them (eg. `<use xlink:href="..." />`), which TypeScript doesn't support at this time (and might not in the future). Test plan: I'll test this in liferay-portal, but for local testing, I added a sample TS file and verified that we see formatting/linting errors in there (when present). Lint suppressions also work. We can remove this file in the future, after we have some actually real/used TS in this repo.
Pretty self-explanatory. These are the changes I needed to make to be able to run the "check" task over all of liferay-portal. Checked it at the top level and also inside individual projects. As noted in the comment, we have to turn off some ESLint rules because they trigger false positives, and end up being redundand with the better, type-aware checks done by the TypeScript compiler itself. We're turning off `@typescript-eslint/no-unused-vars` in type-def files because that can easily trigger false positives in files that define ambient types (ie. the just define the types, they don't explicitly export them, and TS makes them globally available transparently).
|
Going to take this out of draft now because testing it in liferay-portal, it works well. Just need to remember to fix the actual issues in the previous comment, and also kill off this file (which has empty globs in it; which prevent us from actually linting the TS in that project). |
wincent
added a commit
that referenced
this pull request
Oct 1, 2020
This is a crude script that will almost certainly have to be evolved as
the shape and structure of the monorepo evolves, but it works for now
with the set of packages that we currently have.
It addresses a bunch of "gotchas" that come with trying to test out
packages with local changes in a liferay-portal checkout. Depending on
which packages you want to test and the dependency graph between them,
you may find that `yarn link` (or even `yarn add`) doesn't do what you
want. Even if you manually copy files (`cp`), things still might not
work because Gradle may decide to re-run a task that ends up blowing
away any changes you make locally under `node_modules`.
At least for the simple case, this script was good enough to allow me to
test:
#97
even though it involved changes to both `eslint-config` and
`npm-scripts`.
As you can see, it does three things:
- Blows away nested `node_modules` folders inside the monorepo, because
`yarn add` can end up copying these over in ways that cause confusion
(broken `bin` links and so on).
- Cleans out the existing versions of the dependencies (in my testing,
removing and then re-adding seemed to produce a cleaner result than
just overwriting).
- Adds the modules to be tested.
Like I said at the start, this is a hack (and not even trying to make it
work on Windows), but it's a useful hack.
|
Whoops; accidentally posted this comment on the wrong PR... meant to do this here. I'm going to take these Slack reactions as "proof" that people have looked at this:
I want to merge it so that I can build #92 on top of it without having merge conflicts. Happy to follow-up with fixes or modifications for any feedback that comes in. |
wincent
added a commit
that referenced
this pull request
Dec 18, 2020
I guess this is stable enough to document it; the setting is already the recommended default in the canonical "portal-developer.properties" file: https://github.com/liferay/liferay-portal/blob/a0efb8c930d3246f80cc349dba4f0c85be8a4768/portal-impl/src/portal-developer.properties#L22 Initial feedback on the performance and reliability of the feature can be found in this #guild-frontend-es thread (in Spanish 😂): https://app.slack.com/client/T03BTCQAJ/C25TXMD63/thread/C25TXMD63-1569956211.028100 Closes: liferay/liferay-frontend-guidelines#97
wincent
added a commit
that referenced
this pull request
Dec 18, 2020
docs: document "deployFast" (#97)
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.
Note that the goal here is to format and lint TypeScript in liferay-portal, but we're "dog-fooding" in this monorepo already in this PR, even though there isn't any TypeScript in here (yet... when we bring across the js-toolkit packages TypeScript will start coming in here too).
Fun fact, we're so forward-thinking 😂 that we already have ".ts" and ".tsx" in our list of lintable globs and it's been that way for 16 months now — it's just that we never configured our globs in liferay-portal to actually target TS files, because there weren't any yet (and that's also why we never added the necessary deps:
@typescript-eslint/parserand@typescript-eslint/eslint-plugin).Note how we can lint JS just fine with the TS parser.
Closes: #96