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: revert import config module as data #13731

Merged
merged 2 commits into from Jul 6, 2023

Conversation

patak-dev
Copy link
Member

Description

Fixes #13730. See issue for description. If we want to move forward with using data:text/javascript, we would need to consider more edge cases. I think reverting is the best move right now.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@stackblitz
Copy link

stackblitz bot commented Jul 6, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@patak-dev patak-dev added the p4-important Violate documented behavior or significantly improves performance (priority) label Jul 6, 2023
@patak-dev patak-dev merged commit b0bfa01 into main Jul 6, 2023
20 checks passed
@patak-dev patak-dev deleted the fix/revert-import-config-module-as-data branch July 6, 2023 14:11
@silverwind
Copy link

silverwind commented Jul 6, 2023

Should re-open these:

#13267
#9470

@patak-dev
Copy link
Member Author

Done, thanks @silverwind!

@silverwind
Copy link

Thanks, could you also unlock the discussions in them?

@s10y10
Copy link
Contributor

s10y10 commented Jul 7, 2023

I'm sorry for the situation, let's find a better way

@patak-dev
Copy link
Member Author

Dont worry @s10y10, it was good we tried the approach.

@silverwind
Copy link

silverwind commented Jul 7, 2023

I think the revert was premature, the issue should have been analyzed at least.

#13730 did not provide a reproduction, so we will never know what the actual issue was (strictly speaking, the issue is invalid because it has no reproduction).

Maybe the URL was somehow incorrectly encoded, e.g. stuff that good test coverage can catch.

@patak-dev
Copy link
Member Author

@silverwind I think the revert was valid here, we weren't sure this approach was safe and it failed as soon as we released it, and seems the issue is on our side. We can try with more time again. I hope @cpojer could provide a minimal reproduction to help you test a new take at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[4.4.0] "Invalid URL" error with dynamic imports in plugins.
4 participants