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

do not fail on inline scripts #13

Merged
merged 3 commits into from
Apr 16, 2024
Merged

Conversation

enspandi
Copy link
Contributor

Thank you for this great addon - just what I was looking for after the popular webpack plugin didn't work ;)

Hope this fix makes sense as I got an error with an inline script (no src):

- broccoliBuilderErrorStack: TypeError: Cannot read properties of null (reading 'replace')
    at ~/my-project/node_modules/.pnpm/webpack-subresource-integrity-embroider@0.2.1/node_modules/webpack-subresource-integrity-embroider/index.js:37:44
    at Array.map (<anonymous>)
    at ~/my-project/node_modules/.pnpm/webpack-subresource-integrity-embroider@0.2.1/node_modules/webpack-subresource-integrity-embroider/index.js:29:48

if (!assetLocation) {
// skip inline scripts
return;
}
Copy link
Contributor Author

@enspandi enspandi Apr 15, 2024

Choose a reason for hiding this comment

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

Imo. this is mostly for catching and ignoring inline scripts. Having a link element without a href attribute doesn't sound very useful.

But in both cases I think it's safe to ignore integrity hash calculation...

Copy link
Owner

@jelhan jelhan left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Do you mind adding another test app, which covers the scenario of inline scripts?

Comment on lines +19 to +24

<script></script>
<script type="module"></script>
<script type="text/javascript"></script>
<script type="application/javascript"></script>
<script type="application/json"></script>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test app is like the default one - just this part is different

@enspandi enspandi requested a review from jelhan April 16, 2024 08:21
@jelhan
Copy link
Owner

jelhan commented Apr 16, 2024

@enspandi Lockfile seems to be out of sync.

Copy link
Owner

@jelhan jelhan left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@jelhan jelhan merged commit 7928b0e into jelhan:main Apr 16, 2024
2 checks passed
@jelhan jelhan added the bug Something isn't working label Apr 16, 2024
@jelhan jelhan changed the title skip inline scripts do not fail on inline scripts Apr 16, 2024
@jelhan
Copy link
Owner

jelhan commented Apr 16, 2024

Released as v0.2.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants