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: windows build #17

Merged
merged 1 commit into from
Jul 29, 2021
Merged

fix: windows build #17

merged 1 commit into from
Jul 29, 2021

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented Jul 29, 2021

This PR fixes the ESM build in windows.

There were two issues with the specificities of paths in Windows that were not allowing windows builds:

  • File URL path must not include encoded \ or / characters
  • Cannot import ./indexjs from /esm/_ipjsInput.js

The first was basically because we were creating the following type of URL, which was problematic on its encodings.

URL {
  href: 'file:///C:%5CUsers%5CRUNNER~1%5CAppData%5CLocal%5CTemp%5Cf0cb966061a76cc867b77cbb3dd07e18/package.json',
  origin: 'null',
  protocol: 'file:',
  username: '',
  password: '',
  host: '',
  hostname: '',
  port: '',
  pathname: '/C:%5CUsers%5CRUNNER~1%5CAppData%5CLocal%5CTemp%5Cf0cb966061a76cc867b77cbb3dd07e18/package.json',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

Error:

TypeError [ERR_INVALID_FILE_URL_PATH]: File URL path must not include encoded \ or / characters

The second error was due to the windows relative path obtained in windows came as './src\\index.js', meaning the generated code created was:

import './indexjs' from `./esm/_ipjsInput.js`

Resolves #15

@vasco-santos
Copy link
Contributor Author

An example of a build working on Linux, MacOS and Windows with this PR: ipfs/aegir#865

It would be great to extend the tests here to support Windows and avoid regressions, but https://github.com/mikeal/estest does not support windows. We should open an issue to add such tests as follow up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows support
2 participants