feat!: remove unsupported specs from NodeJS export #248
Conversation
| @@ -0,0 +1,18 @@ | |||
| import type { JSONSchema7 } from 'json-schema'; | |||
|
|
|||
| declare module '@asyncapi/specs' { | |||
There was a problem hiding this comment.
Are we gonna remove index.js in favor of this?
There was a problem hiding this comment.
but then we need transpilation TS -> JS process.
There was a problem hiding this comment.
Yes, otherwise we manually maintain both versions.
There was a problem hiding this comment.
Can be done, lets wait for others opinion.
There was a problem hiding this comment.
yeah, I definitely prefer we generate index.js as it doesn't make sense to manually duplicate
There was a problem hiding this comment.
Are we gonna do this in this PR or in another one right after merging this one?
7b63e79 to
98ea22b
Compare
package-lock.json
Outdated
| @@ -8342,4 +8342,4 @@ | |||
| "dev": true | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
it is strange that you added new dependency, but this is the only change in package-lock 🤔
There was a problem hiding this comment.
agreed - I think at least @types/json-schema should've been added here
README.md
Outdated
| @@ -1,18 +1,25 @@ | |||
|   | |||
|
|
|||
| > If you are currently using version 2, check out [migration guideline to version 3](./migrations/Migrate%20to%20version%203.md). You might be able to update it without any change. | |||
There was a problem hiding this comment.
we need to update this part a bit.
we talk about migration from 2 ->3 but then there is v4, so might be confusing
maybe we need dedicated section, like Migrations, where we list path from 2 -> 3 and explain that 3 -> 4 means that we no longer support versions prior 2.0 which means migration path is to switch to 2.x version of the spec
package-lock.json
Outdated
| @@ -8342,4 +8342,4 @@ | |||
| "dev": true | |||
| } | |||
| } | |||
| } | |||
There was a problem hiding this comment.
agreed - I think at least @types/json-schema should've been added here
index.d.ts
Outdated
| import type { JSONSchema7 } from 'json-schema'; | ||
|
|
||
| declare module '@asyncapi/specs' { | ||
| declare const specs: { |
There was a problem hiding this comment.
should we have the declare here?
when I tried opening this, I got an error:
A 'declare' modifier cannot be used in an already ambient context.ts(1038)
I've not created typescript declarations before, so I'm not really sure what the implications of this are - but removing the inner declare made the IDE error go away. 🤷
There was a problem hiding this comment.
hmm, strange. I wasn't getting that error, but I removed that keyword declare before const. On the stackoverflow people write that since some version of TS the word declare is not needed when we declare it inside a module.
Thanks!
|
FYI, this PR is blocking #264. See Lukasz' comment. |
|
@magicmatatjahu pingy pongy |
|
@derberg Done :) |
There was a problem hiding this comment.
Due to the fact this PR blocks #264 (See #264 (comment)), I suggest we first merge this one, we then enable TS transpilation asap in another PR
|
Kudos, SonarCloud Quality Gate passed!
|
|
@magicmatatjahu wanna do the honors? 😄 |
|
@fmvilas Sure! :) |
|
/rtm |
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Readme.mdRelated issue(s)
Resolves #238