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

file extension issues #795

Closed
ghost opened this issue Jul 24, 2016 · 33 comments
Closed

file extension issues #795

ghost opened this issue Jul 24, 2016 · 33 comments

Comments

@ghost
Copy link

ghost commented Jul 24, 2016

It seems like latest version of Rollup force the developer to add a file extension

import x from 'x.js'

and not the normal way

import x from 'x

Any reason for this? I also noticed the Rollup source does it this way.

This practise breaks a lot of things. E.g. if you working with TypeScript and first need to transpile all source code down to ES6. The transpilled source will now have '.ts' extension.

If you now try to bundle the transpilled source with Rollup you get issues because the typescript files isn't found.

There are other scenarios too like this. And in the TypeScript example there isn't an option itself to use that plugin. Way to buggy. And if you try to use that plugin, you will also notice that file extensions are required. As seen in this issue ticket: rollup/rollup-plugin-typescript#52

@ghost
Copy link
Author

ghost commented Jul 24, 2016

Rolled back to use Rollup v. 0.32.4 solved the issue for now.

@TrySound
Copy link
Member

@KFlash As you can see here extension is not required.

@TrySound
Copy link
Member

Maybe the problem is that you are trying to use not relative paths?

@ghost
Copy link
Author

ghost commented Jul 25, 2016

The funny thing is that everything works as expected until v. 0.33.0. After that version I got issues.
If you look at the issue I linked too, you will see that others as well experience this.
However. I'm using TS files and that is what causing the issue. JS files works as expected.

@ghost
Copy link
Author

ghost commented Jul 25, 2016

@TrySound I'm using windows atm, and I tracked the issue I'm facing to this PR: #760

@ghost
Copy link
Author

ghost commented Jul 25, 2016

@TrySound I tried to revert the changes in this PR locale, and it works just fine! So the issue is with that PR. The issue is located to be in the CLI source code.

This lines:

https://github.com/rollup/rollup/blob/master/bin/src/runRollup.js#L1

https://github.com/rollup/rollup/blob/master/bin/src/runRollup.js#L57-L58

https://github.com/rollup/rollup/blob/master/bin/src/runRollup.js#L83

@TrySound
Copy link
Member

But this lines don't response for resolving in rollup itself.

@ghost
Copy link
Author

ghost commented Jul 26, 2016

I was running it from cli. It worked after I changed the lines. Sounds
crazy. Further crazy is it that something from v 0.33.0 screw this up too.

Easiest to reproduce if you use Windows and try it out. I linked to another
issue ticket earlier. He have it rigged so you can try and fail

On Jul 26, 2016 12:06 PM, "Bogdan Chadkin" notifications@github.com wrote:

But this lines don't response for resolving in rollup itself.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#795 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEU1AqPZd4QIw9LXvVWUnS7jE9tf5i2lks5qZYfXgaJpZM4JTsXM
.

@ghost
Copy link
Author

ghost commented Jul 26, 2016

4 persons experience the same thing with Windows

On Jul 26, 2016 2:27 PM, "Kenny Flashlight" kflash123@gmail.com wrote:

I was running it from cli. It worked after I changed the lines. Sounds
crazy. Further crazy is it that something from v 0.33.0 screw this up too.

Easiest to reproduce if you use Windows and try it out. I linked to
another issue ticket earlier. He have it rigged so you can try and fail

On Jul 26, 2016 12:06 PM, "Bogdan Chadkin" notifications@github.com
wrote:

But this lines don't response for resolving in rollup itself.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#795 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEU1AqPZd4QIw9LXvVWUnS7jE9tf5i2lks5qZYfXgaJpZM4JTsXM
.

@ghost
Copy link
Author

ghost commented Aug 2, 2016

Looks like this issue isn't going to be fixed? For me it's an annoying issue on Windows, so I did some digging into the source code and found out why this issue occur!

Note! All files except .js are effected both on Mac and Windows.

This function https://github.com/rollup/rollup/blob/master/src/utils/defaults.js#L9 is the evil one!

If you do this, it will automatically add the .js file extension.

// import
import a from './rollup';

// will search for rollup.js

What if this is an TypeScript file, or CoffeScript or other file types? A error will be thrown because the file isn't found.

And also sometimes I'm still forced to use the .js extension on imported files with Rollup.
This will cause issues with the TS compiler....

@ghost
Copy link
Author

ghost commented Aug 2, 2016

There are still a issue also with Rollup v. 0.33.1 on Windows when using the CLI. Crazy enough not on all Windows computers I tested it on, but a few.

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

@KFlash

  • fs.stat does not create file. So nothing is evil here.
  • typescript plugin has own resolver, so this part of code has no effect on your results.

@ghost
Copy link
Author

ghost commented Aug 2, 2016

@TrySound file += '.js'; this one will add the js extension if there is no file extension.

It uses the function isFile() to figure out if it's an valid file or not. If not, it add the extension.

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

Yep. It's fallback. I meant it does not create new one with that extension.

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

It's only default behavior.

@ghost
Copy link
Author

ghost commented Aug 2, 2016

I know, so if you import a TypeScript file like this

import a from './rollup'

it will start to search for rollup.js and not rollup.ts and throw an error.

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

Yep. Is that a problem for you? What is the reason to let rollup by default look for some weird ts extension? If you will look at rollup source code see we even do not use fallback for js and will remove support of it soon.

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

try

import a from './rollup.ts'

@ghost
Copy link
Author

ghost commented Aug 2, 2016

If you try to test this on Rollup v. 0.32.4 it works just fine. Changed happened after 0.33.0.

Well. If you look at the issues on the TS plugin repo you will see that this is an issue for many users.

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

Well without ts plugin it shoudn't look for ts extension. So the problem with ts plugin itself.

@ghost
Copy link
Author

ghost commented Aug 2, 2016

In my case I'm not using that plugin. But I'm trying to figure out what changed in earlier versions

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

And how do you process typescript? Own realisation?

@ghost
Copy link
Author

ghost commented Aug 2, 2016

TSC

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

Can you provide some repo with repro? And we will figure out together.

@ghost
Copy link
Author

ghost commented Aug 2, 2016

I will if I can't solve this. I will come back to this issue soon.

@ghost
Copy link
Author

ghost commented Aug 2, 2016

Solved it for now by adding this expensive code snippet

file += '.ts';
 if ( isFile( file ) ) return file;

But I guess Webpack, Browserify etc do this different.

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

Show your resolver

@ghost
Copy link
Author

ghost commented Aug 2, 2016

I simply let it check for TS files after JS files. But a solution to this issue could be so simply that Rollup check against the most common file extensions. If not - instead of returning null - throw an error suggestion to use a Rollup plugin.

Would be better then just throw this: Error: Could not load null: path must be a string or Buffer. Then at least users knows what's wrong

@TrySound
Copy link
Member

TrySound commented Aug 2, 2016

Rollup do it. For default. Throw error by yourself. This will prevent using the next resolver including default.

@ghost
Copy link
Author

ghost commented Aug 2, 2016

NP. Others who have some issues can continue this discussion. In fact I stopped using Rollup 1 week ago, and encountered this and some other stuff again when I tried Rollup.

@TrySound TrySound closed this as completed Aug 2, 2016
@ghost
Copy link
Author

ghost commented Aug 2, 2016

For further reading: rollup/rollup-plugin-typescript#52

@fabiosantoscode
Copy link

fabiosantoscode commented Feb 17, 2019

@nifgraup found this issue while converting Terser to ES modules.

For example, in this file: https://github.com/terser-js/terser/blob/master/main.js

I haven't checked the others but lib/compress.js really has to have the .js there.

I am trying to clean up Terser code, so I made a folder lib/compress and put the first file there, index.js. Rollup still couldn't find lib/compress without the /index.js.

@nifgraup
Copy link

@fabiosantoscode The only issue with rollup I found during that task was #2606, in fact I think importing by full file name is clearest.

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

No branches or pull requests

4 participants