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 Vite build bundling error about EISDIR on new URL('.', import.meta.url) #637

Merged
merged 1 commit into from Mar 18, 2022

Conversation

fend25
Copy link
Contributor

@fend25 fend25 commented Mar 18, 2022

Hi!
Vite shows EISDIR error at new URL('.', import.meta.url) expression on build.
Actually checking import.meta.url is not enough to fix this problem.
The problem is when bundler tries to build the code in production build, in compile time it has no idea about how the expression new URL('.', import.meta.url) should be resolved in runtime. And it tries to read all the file and bake it inline in base64 form. And in case of new URL('.', ...) it tries to read the dir what is impossible and throws compile-time error EISDIR.

So I created a workaround code which gives the same output in Vite and Node.js environments:

const filePathname = new URL(import.meta.url).pathname
const folderPathname = filePathname.substring(0, filePathname.lastIndexOf('/')+1)

actually does literally the same as

new URL('.', import.meta.url).pathname

Regarding webpack - it's a bit more complicated.

Now webpack compiles new URL('.', import.meta.url).pathname to something like /1247de54f5a6c6dbd.js.
Obviously it doesn't make sense still '.' clearly describes desired result as a path to some folder. So current webpack behaviour is useless still it doesn't help to get a path to the package folder.

But this is an obvious and strange workaround from the webpack. Because without such config option, webpack just compiles whole path to the lib folder on the compiling machine. There is a closed PR and such option may be switched on in webpack config:

module: {
  parser: {
    javascript: {importMeta: false}
  }
}

With that option, workaround works really good and really provides a path to the folder (actually a plain '/', but it makes sense and it is the desired behaviour of new URL('.', import.meta.url).pathname expression).

Without that option it compiles to something like `/Users/user/projects/project/node_modules/@polkadot/x-global'.

Also closes polkadot-js/extension#1018

BTW, @jacogr, could you please describe what the purpose of this code at all? I'm not sure why packages should provide information about themselves not via standard mechanics of import/require?

…ta.url)`

Hi!
Vite shows EISDIR error at `new URL('.', import.meta.url)` expression on build.
Actually checking import.meta.url is not enough to fix this problem.
The problem is when bundler tries to build the code in production build, in compile time it has no idea about how the expression `new URL('.', import.meta.url)` should be resolved in runtime. And it tries to read all the file and bake it inline in base64 form. And in case of `new URL('.', ...)` it tries to read the dir what is impossible and throws compile-time error EISDIR. 

 So I created a workaround code which gives the same output in Vite dev and Node.js environments:

```js
const filePathname = new URL(import.meta.url).pathname
const folderPathname = filePathname.substring(0, filePathname.lastIndexOf('/')+1)
```

actually does literally the same as 

```js
new URL('.', import.meta.url)
```

Also closes polkadot-js/extension#1018
@jacogr
Copy link
Member

jacogr commented Mar 18, 2022

On bundlers, e.g. webpack, having the dfjkhadsgkjadsgh.js aka gobbelty-gook path is fine.

The path is used in detect - so it shows you exactly where duplicate packages are, without it you see a conflict, but have absolutely no information as to where it is and how to solve it.

@fend25
Copy link
Contributor Author

fend25 commented Mar 18, 2022

But it's strange...
Since Webpack provides a strange and not desired behaviour, but which allows to work duplicate-avoiding logic (which is already are on bundler's shoulders as I understand the process), such logic just drops Vite away at all, and it's a compile time error from a default bundled module which can not be configured.

How can we make @polkadot/* packages family possible to compile via Vite or other bundlers, and not via Webpack only?
Does provided solution look good to you?

@jacogr
Copy link
Member

jacogr commented Mar 18, 2022

The above is a Vite issue, it should handle import.meta.url correctly. Make no mistake, Webpack is far from perfect, i.e. v4 requires hacks and extra config to make it sort-of work.

TL;DR Bundlers are generally quite slow in making progress...

Have not had a gap to look at the suggested solution as of yet.

@fend25
Copy link
Contributor Author

fend25 commented Mar 18, 2022

It is, and as you've mentioned, to merge a PR to bundler is a long-term work.
Could you please take a look at the code change, when you will have time? It holds me a bit because I do a project which is based on top of Vite and cannot build it in production. I appreciate your attention.

Copy link
Member

@jacogr jacogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to test it against a built API (verifying there) against a build webpack instance and it seems spot-on. Exceptionally weird as a bundler work-around.

@jacogr jacogr merged commit 84aac71 into polkadot-js:master Mar 18, 2022
@fend25
Copy link
Contributor Author

fend25 commented Mar 18, 2022

So good! Thank you and I really appreciate it!

@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@polkadot/extension-dapp throw errors
3 participants