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

fix: adjust and replace types for use with new release parse5 #173

Conversation

cristofersousa
Copy link
Contributor

@cristofersousa cristofersousa commented Jul 11, 2022

ISSUE: #172

Resolves:

I was having trouble building with TS, because of the new versions of the parse5 library.
Because of this, it was suggested to make the adjustment to reflect the new scenario, basically what was done:

  • Changed the DefaultTreeDocument property type to Document
  • Adjusted the constructApplications property

Links

https://github.com/DefinitelyTyped/DefinitelyTyped/pull/48402/files#diff-107d402a30275251354f4d0116a2942b4b0631205531ad5ee46c602108ab1836R111

PS: This is my first pull request, any misconduct in the creation please let me know so I can adjust and follow the appropriate team standard.

@cristofersousa
Copy link
Contributor Author

@joeldenning @filoxo could you help me with this code review

Copy link
Contributor

@filoxo filoxo left a comment

Choose a reason for hiding this comment

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

LGTM as far as I could tell!

@timglenn123
Copy link

timglenn123 commented Jan 6, 2023

Hi I am receiving this error as well. Will this update be merged and released anytime soon?

node_modules/single-spa-layout/dist/types/isomorphic/constructRoutes.d.ts:74:81 - error TS2694: Namespace '"/Users/Developer/Workspace/root/node_modules/parse5/dist/index"' has no exported member 'DefaultTreeDocument'.

74 export type RoutesConfig = InputRoutesConfigObject | Element | import('parse5').DefaultTreeDocument | string;

Copy link
Member

@joeldenning joeldenning left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. In the future, please add typescript tests - this project uses tsd to test things

@joeldenning joeldenning merged commit c1b564f into single-spa:main Jun 16, 2023
@joeldenning
Copy link
Member

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.

4 participants