-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Feature: check empty string reference for other resource in HTML(and SVG) #7318
Feature: check empty string reference for other resource in HTML(and SVG) #7318
Conversation
|
Should we error on this, or should we just ignore empty strings? We would currently ignore if the attribute is not there entirely. Maybe someone has a reason for doing this? |
Co-authored-by: Devon Govett <devongovett@gmail.com>
Browsers don't ignore empty src attributes either: |
According to whatwg, a reference to the other resource such as I think it is also reasonable to ignore empty string href, just as Parcel ignores the attribute which is not there entirely. On the other hand, I also think that giving an error on an invalid href will lead more kindful Developer Experience, what do you think about this? FYI: https://html.spec.whatwg.org/multipage/semantics.html#attr-link-href |
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.
Makes sense! Thanks.
This is presenting me warnings like these ones:
But according to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Input/checkbox both these values can be omitted:
and
(example uses an empty |
↪️ Pull Request
Fixes: #7076
Hi team 👋
I investigated about #7076, and I found this is caused by an empty string
uniqueKey
inAsset
.uniqueKey
’s default value is an empty string “”, and this and asset with empty string specifier breaks dependency graph at https://github.com/parcel-bundler/parcel/blob/v2/packages/core/core/src/AssetGraph.js#L517-L519. This is the cause of the strange error.Of course, If I set a string other than an empty string to uniqueKey, the dependency graph will be built as expected, but this empty string reference will be resolved as
/
path and causes another strange behavior.After consideration, I fixed this issue via checking whether or not a reference value in HTML (and also SVG) is empty string in
Transforming
asSassTransformer
checks whether the imported file exists.If there is some context I missed or you have some opinion about this approach, I will be happy if you point it out or feel free to close this PR.
💻 Examples
🚨 Test instructions
I made a test case in
html
in integration tests to checkparcel
correctly throw an error or not with html like an above example.✔️ PR Todo