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

Community contribution required: Typescript Compilebreak after updating from nodejs 8 to 12 #320

Closed
ceisele-r opened this issue Dec 27, 2019 · 24 comments

Comments

@ceisele-r
Copy link

We used sanitize-html in a typescript project with node 8.
After upgrading to node 12, the following typescript compilebreaks occurr in sanitize-html and its dependencies:

node_modules/@types/domutils/index.d.ts:6:10 - error TS2614: Module '"../../../../../node_modules/domhandler/lib"' has no exported member 'DomElement'. Did you mean to use 'import DomElement from "../../../../../node_modules/domhandler/lib"' instead?

6 import { DomElement } from "domhandler";
           ~~~~~~~~~~

node_modules/@types/htmlparser2/index.d.ts:17:10 - error TS2614: Module '"../../../../../node_modules/domhandler/lib"' has no exported member 'DomElement'. Did you mean to use 'import DomElement from "../../../../../node_modules/domhandler/lib"' instead?

17 export { DomElement, DomHandlerOptions, DomHandler, Element, Node } from 'domhandler';


node_modules/@types/sanitize-html/index.d.ts:17:10 - error TS2305: Module '"../../../../../node_modules/htmlparser2/lib"' has no exported member 'Options'.

17 import { Options } from "htmlparser2";

In the package.json, we are using

"sanitize-html": "^1.20.1",

and

"@types/sanitize-html": "^1.20.2",

for the typings.

Could anyone else update successfully to node 12?

Side-note - maybe it helps:
It seems the dependency htmlparser2 was ported to typescript in fb55/htmlparser2@759b122 which was released in https://github.com/fb55/htmlparser2/releases/tag/v4.0.0. Still sanitize-html pulls in "htmlparser2": "^3.10.0", as dependency.

@ceisele-r ceisele-r changed the title Compilebreak after updating from nodejs 8 to 12 Typescript Compilebreak after updating from nodejs 8 to 12 Dec 27, 2019
@ceisele-r
Copy link
Author

I just noticed that domhandler 3.0.0 is also typescript and therefore those types are used instead of @types/domhandler because domutils 2.0.0 pulls in domhandler ^3.0.0.

So since another dependency pulls in htmlparser2 4.0.0 that pulls in domutils ^2.0.0, domhandler ^3.0.0 is pulled in which overwrites the domutils 1.7.0 version pulled in parallel by sanitize-html.

So I think if sanitize-html would also upgrade the htmlparser2 dependency to ^4.0.0, the issue would be solved.

@boutell
Copy link
Member

boutell commented Jan 2, 2020 via email

@boutell
Copy link
Member

boutell commented Feb 24, 2020

I did upgrade to htmlparser2 4.x, and it works great and all tests pass etc., but one user has reported a similar issue since the upgrade. I need a simple procedure to reproduce this issue. Ideally a failing test case PR.

@dmattia
Copy link

dmattia commented Feb 24, 2020

The upgrade doesn't look like it enforces htmlparser2 4.x, the relevant line of code is:

https://github.com/apostrophecms/sanitize-html/blob/master/package.json#L30

@boutell
Copy link
Member

boutell commented Feb 24, 2020 via email

@JamesMcMahon
Copy link

JamesMcMahon commented Mar 19, 2020

This is still broken for me, post upgrade I see the following issues:

 ERROR  Failed to compile with 3 errors                                                                                                                                  5:18:41 PM

 error  in /test-project/node_modules/@types/domutils/index.d.ts

ERROR in /test-project/node_modules/@types/domutils/index.d.ts(6,10):
6:10 Module '"../../../../../../../test-project/node_modules/domhandler/lib"' has no exported member 'DomElement'. Did you mean to use 'import DomElement from "../../../../../../../test-project/node_modules/domhandler/lib"' instead?
    4 | // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
    5 |
  > 6 | import { DomElement } from "domhandler";
      |          ^
    7 | /***
    8 |  * Append an element after another
    9 |  *

 error  in /test-project/node_modules/@types/htmlparser2/index.d.ts

ERROR in /test-project/node_modules/@types/htmlparser2/index.d.ts(17,10):
17:10 Module '"../../../../../../../test-project/node_modules/domhandler/lib"' has no exported member 'DomElement'. Did you mean to use 'import DomElement from "../../../../../../../test-project/node_modules/domhandler/lib"' instead?
    15 | import { DomHandler } from 'domhandler';
    16 | import * as DomUtils from 'domutils';
  > 17 | export { DomElement, DomHandlerOptions, DomHandler, Element, Node } from 'domhandler';
       |          ^
    18 |
    19 | export interface ParserOptions {
    20 |

 error  in /test-project/node_modules/@types/sanitize-html/index.d.ts

ERROR in /test-project/node_modules/@types/sanitize-html/index.d.ts(18,10):
18:10 Module '"../../../../../../../test-project/node_modules/htmlparser2/lib"' declares 'Options' locally, but it is not exported.
    16 | ///<reference types="htmlparser2"/>
    17 |
  > 18 | import { Options } from "htmlparser2";
       |          ^
    19 |
    20 | export = sanitize;
    21 |

 ERROR  Build failed with errors.

@burtonator
Copy link

Still have the same issue on my end too.

@Half-Shot
Copy link

This is also broken for us (matrix-org/matrix-appservice-irc#1003)

@vladimiry
Copy link

vladimiry commented Mar 30, 2020

Same here, have to use custom / very simplified typings for now.

@kieusonlam
Copy link

Same for me.

@boutell
Copy link
Member

boutell commented Apr 1, 2020

We do not use typescript in house, so this will need to be resolved by a community contributor who is familiar with it. We are waiting for a PR, in other words. Thank you for verifying the issue — but past this point please do not add "me too" comments until there is some indication that an attempt has been made to resolve the issue.

@boutell boutell changed the title Typescript Compilebreak after updating from nodejs 8 to 12 Community contribution required: Typescript Compilebreak after updating from nodejs 8 to 12 Apr 1, 2020
@dsyncerek
Copy link

I've made a PR to @types/sanitize-html DefinitelyTyped/DefinitelyTyped#43870, which should resolve the issue with htmlparser2 types.

@boutell
Copy link
Member

boutell commented Apr 14, 2020

That's great! Since we don't use TypeScript in-house, would someone please check out the PR from @dsyncerek and confirm that it does in fact solve the problem for them as well? cc @kieusonlam @vladimiry @Half-Shot @burtonator

@boutell
Copy link
Member

boutell commented Apr 14, 2020

Actually it sounds like there might not be anything to do on our end?

@dsyncerek
Copy link

yes, only @types/sanitize-html needs adjustments, as the htmlparser2 v4 has been rewritten to TypeScript, so it uses own typings instead of community @types/htmlparser2.

@rico-ocepek
Copy link

rico-ocepek commented Apr 22, 2020

Until @dsyncerek DefinitelyTyped PR is merged I have solved the issues by fixing my project's dependencies to these versions:

{
    ...
    "sanitize-html": "1.21.0",
    "@types/sanitize-html": "1.20.2",
    "@types/node": "10.17.21"   //12.12.6 for node 12
    ...
}

Note: @types/node was required in my case as htmlparser2 seems to have issues with this sanitize-html version in conjunction with my node version - you might be able to skip it

@dsyncerek
Copy link

PR is merged now 🎉

@ceisele-r
Copy link
Author

Thanks @dsyncerek , I can confirm its working not in 1.23.0 with types in 1.23.0.

@boutell
Copy link
Member

boutell commented Apr 27, 2020 via email

@zhaque44
Copy link

zhaque44 commented Nov 7, 2020

Good Afternoon: I tried everything on this thread still having this problem:

node -v
v15.0.1

"dependencies": {
"@apollo/react-hooks": "^3.1.3",
"apollo-cache-inmemory": "^1.6.5",
"apollo-client": "^2.6.8",
"apollo-link": "^1.2.14",
"apollo-link-context": "^1.0.20",
"apollo-link-http": "^1.5.16",
"classnames": "^2.2.6",
"graphql": "^14.6.0",
"graphql-tag": "^2.10.3",
"jsonwebtoken": "^8.5.1",
"lodash-es": "^4.17.15",
"mid-common": "workspace:",
"react": "^16.12.0",
"react-apollo": "^3.1.3",
"react-dom": "^16.12.0",
"react-router": "^5.1.2",
"react-router-dom": "^5.1.2",
"sanitize-html": "^2.0.0",
"semantic-ui-react": "^2.0.0"
},
"devDependencies": {
"@types/classnames": "2.2.10",
"@types/eslint": "6.1.8",
"@types/jest": "^26.0.15",
"@types/jsonwebtoken": "8.3.9",
"@types/lodash-es": "4.17.3",
"@types/react": "^16.9.19",
"@types/react-dom": "^16.9.7",
"@types/react-router": "5.1.4",
"@types/react-router-dom": "5.1.3",
"@types/sanitize-html": "1.27.0",
"apollo": "^2.22.0",
"current-git-branch": "^1.1.0",
"eslint": "^6.8.0",
"jest": "^26.6.1",
"minimatch": "^3.0.4",
"ts-jest": "^23.10.5"
},
"jest": {
"transform": {
"^.+\.tsx?$": "ts-jest"
},
"testRegex": "(/tests/.
|(\.|/)(test|spec))\.(jsx?|tsx?)$",
"moduleFileExtensions": [
"ts",
"tsx",
"js",
"jsx",
"json",
"node"
]
}
}

ERROR:

TS2614: Module '"../../../../../../../Desktop/workflow-ui/node_modules/domhandler/lib"' has no exported member 'DomElement'. Did you mean to use 'import DomElement from "../../../../../../../Users/a-zhaque/Desktop/workflow-ui/node_modules/domhandler/lib"' instead?

@abea
Copy link
Contributor

abea commented Nov 9, 2020

@zhaque44 That error doesn't look like it's referencing this package.

@VigneshPT
Copy link

VigneshPT commented Nov 11, 2020

I still get the error

node_modules/dom-serializer/lib/index.d.ts:1:13 - error TS1005: '=' expected.

 import type { Node } from "domhandler";

in the typecheck step tsc -p tsconfig.json --noEmit

I'm using "^2.1.2" version of sanitize-html with "@types/sanitize-html": "^1.27.0". Also "@types/node": "8.10.36", and "typescript": "3.1.3" as dependency. Does any one have a clue?

Thanks in advance.

@GuiMoraesDev
Copy link

I'm facing the same problem

Node version: 12.18.3

"sanitize-html": "^2.1.2",
"@types/sanitize-html": "^1.27.0",
"typescript": "~3.7.2",
"@types/node": "^12.0.0",
"react": "^16.13.1",
/app/node_modules/@types/sanitize-html/node_modules/dom-serializer/lib/index.d.ts

TypeScript error in /app/node_modules/@types/sanitize-html/node_modules/dom-serializer/lib/index.d.ts(1,13):

'=' expected.  TS1005

import type { Node } from "domhandler";
                   ^
export interface DomSerializerOptions {
   emptyAttrs?: boolean;
   selfClosingTags?: boolean;

@abea
Copy link
Contributor

abea commented Nov 17, 2020

Are these not issues coming from @types/sanitize-html's wrapper? We don't use or maintain the typescript version ourselves. By the looks of it, @types/sanitize-html is based on sanitize-html@1.27.0, so mixing that with directly installed v2.1.2 doesn't seem right anyway.

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

No branches or pull requests