Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Fixing imports from acorn and resolve #236 #237

Closed
wants to merge 1 commit into from

Conversation

haraldrudell
Copy link

@lukastaegert
Copy link
Member

Hi @haraldrudell, thanks for the PR, you're certainly right about acorn. However I do not understand why you changed the import for resolve. Resolve itself is not an ES6 module and I think it is this change which is breaking the CI build.

@haraldrudell
Copy link
Author

haraldrudell commented Oct 7, 2017

I should not have consumed this code to begin with. It appears it is not being used in my environment and perhaps should be removed? Usually, this is consumed as CommonJS, in this case, it was being loaded as an ECMAScript module.

I was just acting on the warnings produced by Rollup. As far as I could tell both those imports where inaccurate per ECMAScript 2015. Rollup converts CommonJS modules to ECMAScript modules.

If some non-standard transpile or framework was being used for imports, or those imports were not actually being used, that might explain why the code could have appeared to be working. Or I could be wrong and the CommonJS plug-in buggy…

I only spent minutes to make Rollup complete successfully. As soon as I ran the thing, it was clear I had a configuration problem. It was pretty scary for a minute, because this problem blocked Rollup itself from loading, which is why I made the pull request.

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.

2 participants