-
-
Notifications
You must be signed in to change notification settings - Fork 236
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
refactor: Be stricter about imports #485
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
473363a
Add missing fields to `package.json`s
fb55 b895ada
Move `doctype.serializeContent` to htmlparser2 adapter
fb55 d44dde7
Don't import unicode file outside of parse5
fb55 7d8a47a
Simplify feedback sim `onEndTag`
fb55 76eeaf4
Add several exports to parse5 module
fb55 ae51d98
Disallow duplicate imports, enforce type imports
fb55 da7849a
Move `hasUnescapedText` to `common/html`
fb55 01c42e2
Export `html` sub-module, rename `NAMESPACES`
fb55 6753973
Export `escapeString`
fb55 3de96c4
Add missing import
fb55 6622a3b
Export defaultTreeAdapter, ParserError
fb55 b10c394
Update missed deep parse5 imports
fb55 df1abe8
Update parser-feedback-simulator.ts
fb55 e2163a4
Prefer importing properties from parse5 module
fb55 092db15
Export `Token`
fb55 47e92d2
Remove wildcard export map entry
fb55 9f6f973
Merge branch 'master' into stricter-imports
fb55 5ca70c3
Fix merge artifacts
fb55 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by setting this, does that mean importing an exact file will fail in typescript? since it'd look for the definition in here rather than
{whatever-you-imported}.d.ts
. maybe thats your intention? still trying to understand the overall change tbh so could've just misunderstoodThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this PR, exact file imports are no longer allowed — all you can do is import the modules. This is done through the
exports
field.As for the
types
field — from my understanding, this is for TS to figure out how to resolve a plain module import (eg.import ... from 'parse5'
). TS should use co-located.d.ts
files when you actually import a file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because we don't bundle anything, i feel like we don't need it. typescript will already detect that there's an
index.d.ts
for theindex.js
. using a bare import will resolvemain
or whatever else, and then typescript will find the paralleld.ts
file iirc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am honestly not sure if we need it or not. Eg.
tsc
might be able to pick up the type definitions, but will NPM show the TS badge if there is notypes
field in thepackage.json
?My gut instinct is to keep this field, just to be safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just tried it on a package, with
types
set using a direct import of a module. it seems to work fine, so i think you're ok to leave this here & yeah npm checks explicitly for types/typings AFAIK