Skip to content
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

Retire sed #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aleksei-berezkin
Copy link
Contributor

@aleksei-berezkin aleksei-berezkin commented May 18, 2020

Hi. Started here from just replacing sed with couple of scripts, without moving to any other script. The result apidoc === what was before, checked it with per-file diff.

@@ -7,7 +7,7 @@ jobs:
build:
docker:
# specify the version you desire here
- image: circleci/node:8.11.1
- image: circleci/node:12.16.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's to use async generators in scripts

@@ -1,73 +1,35 @@
#!/usr/bin/env bash
set -e

# we'll modify files in src/ and then revert our changes using
# git checkout src/* so make sure they have no local changes
if git status --porcelain | grep 'src/'; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this up and restricted the check to only files in src

}
}

const SUFFIX = 'Glabiboulga';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw, what's this? :)

@@ -10,6 +10,9 @@
"outDir": "./dist",
"declaration": true,
"sourceMap": true,
"lib": ["es2015", "dom"] // I need DOM because of typedoc..
}
"lib": ["es2015", "dom", "esnext.asynciterable"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't check all files after previous contribution bringing this name, and that was a mistake. This time I found out that my IDE got lost and couldn't read any config, so it reported a lot of compilation errors. I don't know what's your favorite IDE but I suppose if you browse a lot you also gonna encounter something strange. I experimented with configs inheritance and found out that, in order to get maximum support from IDE (like code completion, jump to .d.ts), it's better to have base tsfonfig.json which is like “all-in-one”, declaring all libs and covering all files. Then, other configs may inherit from it, reducing libs and sources. So I propose this fix to configs layout.

@@ -10,6 +10,9 @@
"outDir": "./dist",
"declaration": true,
"sourceMap": true,
"lib": ["es2015", "dom"] // I need DOM because of typedoc..
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually ChromeDevToolFormatters use window so I propose removing the comment

@emmanueltouzery
Copy link
Owner

emmanueltouzery commented May 21, 2020

hi, good effort, thank you for the PR! Well, first 'glabiboulga', i mean i needed something to append to differentiate, i could have put anything. I actually mistyped 'gloubi-boulga' which is from an old french kids cartoon :)

For the PR... I feel like what we have for the apidocs publishing currently is a workaround due to some limitations that typedoc had at the time. At the time I felt that typedoc development was not very active and I didn't expect the issues to be fixed relatively quickly (so much so that I didn't even report them).

So we do all this replacing specifically because:

# trick for the 'Option' & 'Either' constants which typedoc skips as it clashes
# with the 'Option' & 'Either' type synomym

So if we didn't rename, typedoc dropped the Option & Either constants, producing a bad apidoc (you can easily reproduce it by just commenting these workarounds).
Then there is a second part to change 'files' to 'modules' but i mean we could also live with the modules terminology possibly.
Then we have more tricks with typedoc, to get grouping for "Control", "Collection", "Core". There is a real chance that those bits will break due to the upgrade, but that part was already typescript, it's probably fixable. We could even possibly give up the feature, or try to achieve a similar effect by doing our own docs intro page linking to the typedoc api instead of hacking the typedoc HTML.

I don't even know how much longer we can hold on such an old version of typedoc, because typescript keeps adding features and that old typedoc doesn't know anything about the new stuff. It could stop being able to parse the source anytime. The typedoc version that prelude currently uses is 0.7.2... The latest is 0.17.7, and the project appears to have picked up steam.

So, rather than officializing the workarounds, I'd try to upgrade typedoc first, see how that looks like. Is the constant vs type synonym issue still present? If so, we could maybe make a little reproduction project, open them a bug, ask them how we should handle this.

What do you think? I'd rather try that first, and see where it takes us.

@aleksei-berezkin
Copy link
Contributor Author

Hi thanks for your feedback. Well if the newer version of the tool produces the correct docs, it would be ideal just to migrate and not having hacks. However as I sad there's some inconvenience with tsconfig.base I'd like to fix probably in different PR. What do you think?

@emmanueltouzery
Copy link
Owner

yes i'm ok to change the tsconfig setup. i'm actually using emacs, this doesn't inconvenience me much, but i'm all for making it easy for others to use!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants