Skip to content

__dirname is broken when page URL starts with file:/// #1711

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.

Sign up for GitHub

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

Jump to bottom
Closed
saschanaz opened this issue Jul 19, 2017 · 8 comments
Closed

__dirname is broken when page URL starts with file:/// #1711

saschanaz opened this issue Jul 19, 2017 · 8 comments

Comments

@saschanaz
Copy link

saschanaz commented Jul 19, 2017 •
edited
Loading

SystemJS in local environment fails to get correct __dirname. It becomes /C:/... when it should be file:///C:/... (absolute) or just / (relative).

On MSEdge:

<!DOCTYPE html>
<html>

<head>
    <meta charset="utf-8" />
    <script src="https://rawgit.com/systemjs/systemjs/master/dist/system.js"></script>
    <script>
        document.addEventListener("DOMContentLoaded", async () => {
            const __dirname = await System.import("systemjsrepro.js");
            h2.textContent = `__dirname: ${__dirname}`
        });
    </script>
</head>

<body>
    <h2 id="h2">__dirname?</h2>
</body>

</html>
// systemjsrepro.js
module.exports = __dirname;
@guybedford
Copy link
Member

file://C:/... is not a valid file URL - it should be file:///C:/...

Try loading that URL and let me know if that works ok?

@saschanaz saschanaz changed the title __dirname is broken when page URL starts with file:// __dirname is broken when page URL starts with file:/// Jul 22, 2017
@saschanaz
Copy link
Author

@guybedford Ah yes, it actually was file:///C:/.... It still gives a wrong value.

@guybedford
Copy link
Member

I'm not sure I follow how that could be possible. The code at https://github.com/systemjs/systemjs/blob/master/src/format-helpers.js#L166 is designed to strip the file:/// part correctly in windows leaving a c:/... path. Perhaps you can help get a replication here.

@saschanaz
Copy link
Author

It seems the wrong isWindows value is the problem. This works: (typeof process !== 'undefined' && typeof process.platform === 'string' && process.platform.match(/^win/)) || (typeof navigator !== "undefined" && typeof navigator.platform === "string" && navigator.platform.match(/^Win/))

@saschanaz saschanaz mentioned this issue Jul 27, 2017
Closed
@guybedford
Copy link
Member

Thanks for spotting that and the PR. Rather than changing the isWindows code to detect the browser environment I think it would be better to alter the stripOrigin function to work with both path styles based on detecting /^file:///[a-z]\:/i.

@saschanaz
Copy link
Author

saschanaz commented Jul 28, 2017 •
edited
Loading

I think the detection itself is robust, the problem is the current stripOrigin function gets wrong isWindows value so it creates Linux-style path by removing only file:// part from file:///.

@guybedford
Copy link
Member

guybedford commented Aug 3, 2017 •
edited
Loading

@saschanaz I understand, but I don't want to be doing OS environment detection in the browser at all in SystemJS.

@guybedford
Copy link
Member

(Hence, why I'm suggesting a more flexible approach with the regex)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Correct isWindows value on browser environment
2 participants
@guybedford @saschanaz