Skip to content

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
Member

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