Fix up config for docs generation #16

ajacksified · 2021-07-23T13:28:56Z

This updates tsconfig and package.json to fix docs site generation.

docs/api/Makefile

package.json

matthieubosquet · 2021-12-15T16:08:39Z

package.json

@@ -32,7 +32,7 @@
      "require": "./dist/index.js",
      "import": "./dist/index.mjs"
    },
-    "./module": "./dist/module/index.mjs"
+    "./error": "./dist/error.mjs"


Suggested change

"./error": "./dist/error.mjs"

I don't think this makes sense. There should be only one point of export in this package.

(in other words, 1 way to import things from this package)

This is here in this, as in all of our libraries, as a way to import directly import x from "@inrupt/errors/filename" when using ES6 modules.

I think we need to do this as an object, so the typescript types work; there's a issue on github on another repo about this, I think solid-client-js

Yeah, you'd need a typesVersions field (like the one I added to solid-client-access-grant-js).

But for such a small package, it doesn't make sense to enable a sub-module import syntax.

It might actually even be a bad idea for most larger libraries (cost/benefits) but that's not the discussion at hand here.

Agree for this library sub-module imports probably doesn't make sense.

I think it's important to maintain consistency with our other libraries - I don't see any significant "cost" to keeping this here.

I think the kind of consistency we should work towards is applying a set of best practices in coding, one of which would be not to expose sub-modules when a package doesn't have any.

I also think that sub-modules only make sense in specific situations and I'm not sure they're curently warranted in any of our packages since they increase complexity and provide little benefit unless I'm missing something.

I also think that sub-modules only make sense in specific situations and I'm not sure they're curently warranted in any of our packages since they increase complexity and provide little benefit unless I'm missing something.

Which is fine, but that's not a discussion for this PR. That is a separate discussion we can have as it affects all of our libraries.

OK, let's schedule that discussion soon 🤞

(also because as it stands, our submodule exports are broken as missing types)

matthieubosquet · 2021-12-15T16:10:00Z

src/error.ts

@@ -16,6 +16,9 @@
 // HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
 // OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
 // SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+// rdfjs/types package throws linter errors
+// eslint-disable-next-line import/no-unresolved


That's weird, never seen that happen elsewhere, might check it out.

Yeah, it's super weird. It doesn't happen in solid-client (the only library I'm aware of which is using this) because it doesn't have the dependency linter.

I'll try to look at it before we make public.

This can be fixed using the docs here: import-js/eslint-plugin-import#1485

So we need to fix this in our base eslint config.

I've said this before, I think that we should consider making our eslint-config-base peerDependencies non version specific (all *):
https://github.com/inrupt/javascript-style-configs/blob/c169edebb71650bd059e3bdc3fe990cb3cb008c5/eslint-config-base/package.json#L23-L34

@matthieubosquet because we're using ^ versions in our peerDependencies, nothing's stopping us from updating. The eslint config only specifies a minimum, which is useful.

Ok, I get that, but it makes us stuck on majors.

What about just starring the two @typescript-eslint dependencies?

We do want to be on the same majors across our libraries, or as close as possible.

matthieubosquet · 2021-12-15T16:10:57Z

tsconfig.json

@@ -69,20 +69,14 @@
        // The source files of everything listed under `exports` in our package.json
        // (i.e. public API's that should be documented) should be listed here:
        "src/index.ts",
-        "src/index.ts",
+        "src/error.ts",


Suggested change

"src/error.ts",

This doesn't need to/shouldn't be here.

Without this here, the docs don't generate on that file.

That's because src/index.ts is excluded on line 83/79

hahaha, this config is such a mess ^^,

We don't need index though, we just need error. Index just re-exports error.

ThisIsMissEm

Looks good, but I think there's some issue still.

ThisIsMissEm · 2021-12-15T16:23:27Z

package.json

@@ -32,7 +32,7 @@
      "require": "./dist/index.js",
      "import": "./dist/index.mjs"
    },
-    "./module": "./dist/module/index.mjs"
+    "./error": "./dist/error.mjs"


I think we need to do this as an object, so the typescript types work; there's a issue on github on another repo about this, I think solid-client-js

package.json

ThisIsMissEm · 2021-12-15T16:25:12Z

tsconfig.json

@@ -69,20 +69,14 @@
        // The source files of everything listed under `exports` in our package.json
        // (i.e. public API's that should be documented) should be listed here:
        "src/index.ts",
-        "src/index.ts",
+        "src/error.ts",


That's because src/index.ts is excluded on line 83/79

ajacksified requested a review from dgbarclay July 23, 2021 13:28

ajacksified marked this pull request as draft December 15, 2021 11:57

ajacksified removed the request for review from dgbarclay December 15, 2021 11:57

ajacksified force-pushed the fix/docs branch from e384ce6 to d361ecd Compare December 15, 2021 12:05

ajacksified marked this pull request as ready for review December 15, 2021 12:09

ajacksified requested review from NSeydoux and ThisIsMissEm December 15, 2021 12:10

matthieubosquet suggested changes Dec 15, 2021

View reviewed changes

vercel bot deployed to Preview December 15, 2021 16:20 View deployment

ajacksified requested a review from matthieubosquet December 15, 2021 16:20

ThisIsMissEm reviewed Dec 15, 2021

View reviewed changes

ajacksified requested a review from a team as a code owner March 16, 2022 14:50

vercel bot had a problem deploying to Preview March 16, 2022 14:50 Failure

github-actions bot had a problem deploying to review March 16, 2022 14:50 Failure

ThisIsMissEm force-pushed the fix/docs branch from bea12f7 to e5217bc Compare March 16, 2022 15:46

vercel bot had a problem deploying to Preview March 16, 2022 15:46 Failure

github-actions bot had a problem deploying to review March 16, 2022 15:47 Failure

ThisIsMissEm force-pushed the fix/docs branch from e5217bc to b2095e3 Compare March 16, 2022 15:59

vercel bot deployed to Preview March 16, 2022 15:59 View deployment

github-actions bot had a problem deploying to review March 16, 2022 15:59 Failure

vercel bot deployed to Preview March 16, 2022 16:03 View deployment

github-actions bot had a problem deploying to review March 16, 2022 16:04 Failure

ThisIsMissEm and others added 5 commits March 16, 2022 17:19

Remove e2e test workflows as this repo doesn't have e2e tests yet

7e2f4b3

Update README.md

e76affc

Fix lint failures

c91a56f

chore: run prettier on all files & fix lint errors

0bb537c

chore: upgrade docs infrastructure & fix documentation builds

36b6a43

ThisIsMissEm force-pushed the fix/docs branch from 6fdf025 to 36b6a43 Compare March 16, 2022 16:20

vercel bot deployed to Preview March 16, 2022 16:20 View deployment

github-actions bot deployed to review March 16, 2022 16:20 View deployment

Fix package name

6774872

vercel bot deployed to Preview March 16, 2022 16:33 View deployment

github-actions bot deployed to review March 16, 2022 16:33 View deployment

Disable workflows for e2e & release as this package is unreleased

96f4e64

vercel bot deployed to Preview March 16, 2022 16:44 View deployment

ajacksified enabled auto-merge (rebase) March 16, 2022 16:47

ThisIsMissEm merged commit d319d8b into main Mar 16, 2022

ThisIsMissEm deleted the fix/docs branch March 16, 2022 16:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix up config for docs generation #16

Fix up config for docs generation #16

ajacksified commented Jul 23, 2021

matthieubosquet Dec 15, 2021

matthieubosquet Dec 15, 2021

ajacksified Dec 15, 2021

ThisIsMissEm Dec 15, 2021

matthieubosquet Dec 15, 2021

ThisIsMissEm Dec 15, 2021

ajacksified Dec 17, 2021

matthieubosquet Dec 17, 2021

ajacksified Dec 17, 2021

matthieubosquet Dec 17, 2021

matthieubosquet Dec 15, 2021

ajacksified Dec 15, 2021

matthieubosquet Dec 15, 2021

ThisIsMissEm Dec 15, 2021

ThisIsMissEm Dec 15, 2021

matthieubosquet Dec 15, 2021

ajacksified Dec 17, 2021

matthieubosquet Dec 17, 2021

ajacksified Dec 17, 2021

matthieubosquet Dec 15, 2021

ajacksified Dec 15, 2021

ThisIsMissEm Dec 15, 2021

matthieubosquet Dec 15, 2021 •

edited

Loading

ajacksified Dec 17, 2021 •

edited

Loading

ThisIsMissEm left a comment

ThisIsMissEm Dec 15, 2021

ThisIsMissEm Dec 15, 2021

Fix up config for docs generation #16

Fix up config for docs generation #16

Conversation

ajacksified commented Jul 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthieubosquet Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

ajacksified Dec 17, 2021 • edited Loading

Choose a reason for hiding this comment

ThisIsMissEm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthieubosquet Dec 15, 2021 •

edited

Loading

ajacksified Dec 17, 2021 •

edited

Loading