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

Load data url with search correctly #51324

Closed
wants to merge 4 commits into from

Conversation

LongTengDao
Copy link
Contributor

This works in browsers (firefox, safari, even chrome):

import(`data:text/javascript,console.log('?');#...`);

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Dec 31, 2023
@LiviaMedeiros
Copy link
Contributor

Compatibility aside, this looks more like a bug in browsers to me.

> import('data:text/javascript,console.log(new URL(import.meta.url).hash);#123')
#123
> import('data:text/javascript;base64,Y29uc29sZS5sb2cobmV3IFVSTChpbXBvcnQubWV0YS51cmwpLmhhc2gpOw==#123')
#123

> import('data:text/javascript,console.log(new URL(import.meta.url).search);?123')
SyntaxError: Unexpected token '?'
> import('data:text/javascript;base64,Y29uc29sZS5sb2cobmV3IFVSTChpbXBvcnQubWV0YS51cmwpLnNlYXJjaCk7?123')
TypeError: Failed to fetch dynamically imported module

> import(`data:text/javascript,console.log('#');`);
SyntaxError: Invalid or unexpected token

If pathname is properly urlencoded (which should be the case whenever we work with URLs), it works just fine:

> import(`data:text/javascript,console.log('%3F');#...`);
?

Might be related: #42504, #42890,

@LongTengDao
Copy link
Contributor Author

LongTengDao commented Jan 2, 2024

I'm a framework author, and I also wanted to find any spec, then failed.

But under my test and analysis, it rarely seems like a bug of browsers, because:

  1. Browsers do so (ignore only hash) together (while only nodejs ignore both search and hash);
  2. If you try data:text/html,...#element-id, the screen scrolling respect the hash, which make this more like a initiative feature, not negligence.

Even if nodejs does not want to respect the hash, it should at least throw an error, not silently run a code trimmed \t \n \r and ?... #..., as if nothing happened, and write it in docs. Ignoring this problem continuing may reconditely cause big bug in userland as time bomb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants