Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

replacing all ES6 imports with CJS require calls #3063

Merged
merged 5 commits into from May 20, 2017

Conversation

patsissons
Copy link
Contributor

resolves #2934
used the following regex patterns to apply the changes in lib:
s/import\s+(\w+)\s+from\s+(.*);\s*$/var $1 = require($2).default;/
s/import\s+(\w+)\s+from\s+(.*)\s*$/var $1 = require($2).default;/

There weren't too many instances to patch, 91 total. ran build and tests, no issues on either end.

Open to suggestions if there are any. This PR is really designed as a quick band-aid patch to allow vis module imports from within webpack.

resolves almende#2934
used the following regex to apply the changes in lib:
s/import\s+(\w+)\s+from\s+(.*);\s*$/var $1 = require($2).default;/
s/import\s+(\w+)\s+from\s+(.*)\s*$/var $1 = require($2).default;/
@@ -1,4 +1,4 @@
import CachedImage from './CachedImage';
var CachedImage = require('./CachedImage').default;
Copy link
Contributor

Choose a reason for hiding this comment

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

CachedImage is an ES6 module because it is exported with ES6 export syntax.
Images is also an ES6 module. export default Images; at the end.

So it's again mixing styles because it's importing CachedImage with a cjs syntax inside an ES6 module.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since you used a regex to search and replace, I suppose it must have happened in other places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll check for these occurrences. For mixed imports I'll modify the export to CJS style, for unmixed imports i'll revert to ES6 exports.

Edit: ignore that first part, no coffee yet this morning.

@AoDev
Copy link
Contributor

AoDev commented May 18, 2017

Thanks for the PR but this has also changed imports where it shouldn't.
I commented in some changes.

@mojoaxel
Copy link
Member

😞

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.

None yet

4 participants