fix: update dependencies #30

MichaelDeBoey · 2021-07-15T15:11:38Z

Fixes #29

MichaelDeBoey · 2021-07-15T15:37:29Z

@wooorm really weird that @types/mdast is working locally, but not in CI 🤔

Error: [typecheck] src/index.ts(90,14): error TS2339: Property 'title' does not exist on type 'Literal & Parent'.
Error: [typecheck] src/index.ts(92,26): error TS2339: Property 'value' does not exist on type 'Content'.
[typecheck]   Property 'value' does not exist on type 'Paragraph'.
Error: [typecheck] src/index.ts(92,41): error TS2339: Property 'url' does not exist on type 'Literal & Parent'.
Error: [typecheck] src/index.ts(97,14): error TS2339: Property 'url' does not exist on type 'Literal & Parent'.
Error: [typecheck] src/index.ts(163,32): error TS2339: Property 'tagName' does not exist on type 'Parent'.
Error: [typecheck] src/index.ts(164,38): error TS2339: Property 'properties' does not exist on type 'Parent'.

wooorm · 2021-07-15T15:40:38Z

a) I meant using @types/mdast instead of @types/unist. To use specific nodes, like Link for links
b) maybe rm -rf node_modules locally and reinstalling will surface the problems

JounQin · 2021-07-15T15:44:10Z

Maybe you should clear cache on locally/CI first.

kentcdodds

👍

kentcdodds · 2021-07-15T16:15:47Z

Whoops, sometimes I approve before reading the comments or looking at CI 🤦‍♂️

Maybe you should clear cache on locally/CI first.

Unfortunately GitHub actions don't have a way to clear the cache 😬

src/index.ts

wooorm · 2021-07-15T16:47:13Z

src/index.ts

@@ -82,19 +82,19 @@ const remarkEmbedder: Plugin<[RemarkEmbedderOptions]> = ({
      }

      const {children} = paragraphNode
-      const node = children[0] as Literal & Parent
+      const node = children[0] as Link | Text


Why the |? It’s just a Link, presumably, right?

L86 is checking if it's a Text

const isText = node.type === 'text'

We return early if it's not Text or if it's an invalid link.
But that seems weird as a node can't be both text & valid link 🤔

@kentcdodds can probably give some more context about this check

Yeah, we only want to find links but we could find just about anything as the first and only child to a paragraph. But now that I think about it, if isValidLink is true, then isText will always be false so we could probably remove the isText completely.

Alternatively, there are a bunch of utilities to narrow these types: https://github.com/ChristianMurphy/unifiedjs.github.io/blob/docs/typescript-guide-and-recipes/doc/learn/node-type-narrowing-in-typescript.md.
You can do this type safe!

But now that I think about it, if isValidLink is true, then isText will always be false so we could probably remove the isText completely.

@kentcdodds We should keep the isText check as we're also transforming text that's not a valid link apparently 🤔 (see https://github.com/remark-embedder/core/runs/3079154136#step:6:36)

src/index.ts

kentcdodds

LGTM 👍

kentcdodds · 2021-07-15T22:26:05Z

Hmm... Looks like we've got some test failures.

MichaelDeBoey · 2021-07-16T00:03:33Z

@kentcdodds Tests are failing because we should keep the isText check as we're also transforming text that's not a valid link apparently 🤔 (see https://github.com/remark-embedder/core/runs/3079154136#step:6:36)

kentcdodds · 2021-07-16T02:03:28Z

Oh right, that's actually the most common use case 😅 https://astexplorer.net/#/gist/e15b6dea43f639b57bfeeb894a4a44bd/d01873eee84788877d955ad972c20500cbd2a630

Good thing we have tests for that!

MichaelDeBoey · 2021-07-16T13:57:14Z

@kentcdodds The GitHub caching problem is still there, but locally all is green

kentcdodds · 2021-07-16T14:38:25Z

Let's see what happens if I just merge

MichaelDeBoey · 2021-07-16T15:39:38Z

@kentcdodds Seems like there's still something wrong 😕
https://github.com/remark-embedder/core/runs/3087181241

kentcdodds · 2021-07-16T22:47:43Z

I'm actually experiencing errors on a fresh clone:

~/Desktop 🦍
$ git clone https://github.com/remark-embedder/core.git
Cloning into 'core'...
remote: Enumerating objects: 186, done.
remote: Counting objects: 100% (63/63), done.
remote: Compressing objects: 100% (48/48), done.
remote: Total 186 (delta 30), reused 26 (delta 10), pack-reused 123
Receiving objects: 100% (186/186), 65.03 KiB | 924.00 KiB/s, done.
Resolving deltas: 100% (92/92), done.
~/Desktop 🦍
$ cd core 
~/Desktop/core (main) 🦍
$ npm run setup
running command with prefix "setup"

> @remark-embedder/core@0.0.0-semantically-released setup
> npm install && npm run validate -s

npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
npm WARN deprecated object-keys@0.2.0: Please update to the latest object-keys
npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated

added 1189 packages, and audited 1190 packages in 20s

165 packages are looking for funding
  run `npm fund` for details

12 vulnerabilities (6 moderate, 6 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
[build] Successfully compiled 1 file with Babel (645ms).
[build] Generating TypeScript definitions
[typecheck] src/index.ts(92,26): error TS2339: Property 'value' does not exist on type 'StaticPhrasingContent'.
[typecheck]   Property 'value' does not exist on type 'Emphasis'.
[typecheck] src/index.ts(97,19): error TS2339: Property 'value' does not exist on type 'Link'.
[typecheck] src/index.ts(163,32): error TS2339: Property 'tagName' does not exist on type 'Parent'.
[typecheck] src/index.ts(164,38): error TS2339: Property 'properties' does not exist on type 'Parent'.
[typecheck] npm run typecheck --silent exited with code 1
--> Sending SIGTERM to other processes..
[test] npm run test --silent -- --coverage exited with code SIGTERM
--> Sending SIGTERM to other processes..
[build] npm run build --silent exited with code SIGTERM
--> Sending SIGTERM to other processes..
[lint] npm run lint --silent exited with code SIGTERM
NPM command npm run setup -- failed with code 1

github-actions · 2021-07-17T13:56:57Z

🎉 This PR is included in version 1.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@wooorm

…ed by @wooorm Given that the actual code generating mdast (such as remark-parse) has used `null` instead of `undefined` for missing fields, because `null` serializes in JSON and thus makes deep-equality tests with fixtures stored in JSON easier, recognizing that it is easier to have only of either `undefined` or `null`, given that TypeScript users might due to the existing types perform a strict equality test against `undefined` and thus introduce bugs such as such as: <remark-embedder/core#30 (review)>, I propose to add `null` as another “missing” value to match the types with reality. Related to: #54547.

MichaelDeBoey added the dependencies Pull requests that update a dependency file label Jul 15, 2021

MichaelDeBoey requested a review from kentcdodds July 15, 2021 15:11

MichaelDeBoey self-assigned this Jul 15, 2021

kentcdodds approved these changes Jul 15, 2021

View reviewed changes

MichaelDeBoey force-pushed the update-dependencies branch from e51be99 to 9a4c1d9 Compare July 15, 2021 16:44

MichaelDeBoey commented Jul 15, 2021

View reviewed changes

src/index.ts Outdated Show resolved Hide resolved

MichaelDeBoey requested a review from kentcdodds July 15, 2021 16:45

wooorm reviewed Jul 15, 2021

View reviewed changes

MichaelDeBoey mentioned this pull request Jul 15, 2021

[mdast] update Resource's title type DefinitelyTyped/DefinitelyTyped#54547

Merged

8 tasks

MichaelDeBoey force-pushed the update-dependencies branch from 9a4c1d9 to 3a4fcb3 Compare July 15, 2021 17:13

kentcdodds approved these changes Jul 15, 2021

View reviewed changes

MichaelDeBoey force-pushed the update-dependencies branch from 70d2440 to 3a4fcb3 Compare July 16, 2021 13:51

fix: update dependencies

b53b309

MichaelDeBoey force-pushed the update-dependencies branch from 3a4fcb3 to b53b309 Compare July 16, 2021 14:00

kentcdodds merged commit 9c8ae04 into main Jul 16, 2021

kentcdodds deleted the update-dependencies branch July 16, 2021 14:38

MichaelDeBoey mentioned this pull request Jul 17, 2021

CI is failing #32

Closed

github-actions bot added the released label Jul 17, 2021

wooorm mentioned this pull request Aug 16, 2021

[mdast]: allow null wherever undefined is allowed DefinitelyTyped/DefinitelyTyped#55174

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update dependencies #30

fix: update dependencies #30

MichaelDeBoey commented Jul 15, 2021 •

edited

Loading

MichaelDeBoey commented Jul 15, 2021

wooorm commented Jul 15, 2021

JounQin commented Jul 15, 2021 •

edited

Loading

kentcdodds left a comment

kentcdodds commented Jul 15, 2021

wooorm Jul 15, 2021

MichaelDeBoey Jul 15, 2021

kentcdodds Jul 15, 2021

wooorm Jul 15, 2021

MichaelDeBoey Jul 15, 2021 •

edited

Loading

kentcdodds left a comment

kentcdodds commented Jul 15, 2021

MichaelDeBoey commented Jul 16, 2021

kentcdodds commented Jul 16, 2021

MichaelDeBoey commented Jul 16, 2021 •

edited

Loading

kentcdodds commented Jul 16, 2021

MichaelDeBoey commented Jul 16, 2021

kentcdodds commented Jul 16, 2021

github-actions bot commented Jul 17, 2021

fix: update dependencies #30

fix: update dependencies #30

Conversation

MichaelDeBoey commented Jul 15, 2021 • edited Loading

MichaelDeBoey commented Jul 15, 2021

wooorm commented Jul 15, 2021

JounQin commented Jul 15, 2021 • edited Loading

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds commented Jul 15, 2021

wooorm Jul 15, 2021

Choose a reason for hiding this comment

MichaelDeBoey Jul 15, 2021

Choose a reason for hiding this comment

kentcdodds Jul 15, 2021

Choose a reason for hiding this comment

wooorm Jul 15, 2021

Choose a reason for hiding this comment

MichaelDeBoey Jul 15, 2021 • edited Loading

Choose a reason for hiding this comment

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds commented Jul 15, 2021

MichaelDeBoey commented Jul 16, 2021

kentcdodds commented Jul 16, 2021

MichaelDeBoey commented Jul 16, 2021 • edited Loading

kentcdodds commented Jul 16, 2021

MichaelDeBoey commented Jul 16, 2021

kentcdodds commented Jul 16, 2021

github-actions bot commented Jul 17, 2021

MichaelDeBoey commented Jul 15, 2021 •

edited

Loading

JounQin commented Jul 15, 2021 •

edited

Loading

MichaelDeBoey Jul 15, 2021 •

edited

Loading

MichaelDeBoey commented Jul 16, 2021 •

edited

Loading