-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Allow usage of mjs #154
Allow usage of mjs #154
Conversation
.eslintrc.cjs
Outdated
@@ -5,7 +5,7 @@ module.exports = { | |||
ecmaVersion: 2020, | |||
project: './tsconfig.eslint.json', | |||
sourceType: 'module', | |||
extraFileExtensions: ['.cjs'], | |||
extraFileExtensions: ['.cjs', '.mjs'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not needed, is it? foundry-factory itself doesn't use .mjs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was undecided on whether to add this or not just because I didn't know if foundry-factory would want to be consistent with the templates it provides. I'll revert this.
@@ -122,7 +122,11 @@ export class GhostGulpRollupPreset implements Preset { | |||
name: 'TypeScript', | |||
value: 'typescript', | |||
}, | |||
{ name: 'Linter & Formatter (ESLint & Prettier)', value: 'linter', checked: true }, | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revert these formatting changes. Applies to other reformatted objects, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... I'm unsure of how they got there in the first place. For sure I wasn't seeking out changes like this, I think it just happened when I ran prettier which is strange, maybe my print width wasn't set wide enough?
@@ -27,13 +28,13 @@ module.exports = { | |||
|
|||
overrides: [{% if useTypeScript %} | |||
{ | |||
files: ['./*.cjs'], | |||
files: ['./*.cjs', './*.mjs'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look right to me: In .mjs
files, we shouldn't allow var requires as they are es modules, so they should use import instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I'd misunderstood what the template was doing trying to put mjs
alongside cjs
, good catch!
I requested #131 in order to fix some problems with Yarn 2/3 and
cjs
but I recently found out thatmjs
may be a better solution.