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

Acknowledge .mjs files #22

Merged
merged 1 commit into from
Jul 16, 2018
Merged

Acknowledge .mjs files #22

merged 1 commit into from
Jul 16, 2018

Conversation

Avaq
Copy link
Member

@Avaq Avaq commented Jul 14, 2018

Similarly to sanctuary-js/sanctuary-scripts#17, this change allows authors to use .mjs files, and still have the Sanctuary tooling lint them by default.

@Avaq Avaq requested a review from davidchambers July 14, 2018 19:20
@@ -101,15 +101,15 @@
}
},
{
"files": ["index.js"],
"files": ["index.{js,mjs}"],
Copy link
Member Author

Choose a reason for hiding this comment

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

If a file ends in .mjs, we could also automatically set the sourceType to module, but I've left that to the user, for now. At least this change makes it so they don't have to go deep into Sanctuary stuff to copy the overrides with small modifications.

Copy link
Member

Choose a reason for hiding this comment

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

If a file ends in .mjs, we could also automatically set the sourceType to module, but I've left that to the user, for now.

This sounds to me like a useful inclusion. Could we specify **/*.mjs in a separate entry to achieve this?

@@ -105,11 +109,11 @@
"globals": {"__doctest": false, "define": false, "module": false, "require": false, "self": false}
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the rule from here. Modules don't have any of these globals.

{
"files": ["*.mjs"],
"parserOptions": {"sourceType": "module"}
},
Copy link
Member Author

Choose a reason for hiding this comment

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

I just tested it, and it seems this rule correctly merges with overlapping rules. :)

Copy link
Member

Choose a reason for hiding this comment

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

Very nice!

@davidchambers davidchambers merged commit ca6128a into master Jul 16, 2018
@davidchambers davidchambers deleted the avaq/mjs branch July 16, 2018 11:40
@davidchambers
Copy link
Member

Shall we release this as v2.1.0 or as v3.0.0?

@Avaq
Copy link
Member Author

Avaq commented Jul 17, 2018

That's a good question. I think the sourceType change could be potentially breaking, although that is very unlikely to happen.

@Avaq
Copy link
Member Author

Avaq commented Jul 17, 2018

very unlikely to happen

In cases where people were using .mjs, having specified override rules for it, but explicitly interpret the file using the "wrong" module system.

@Avaq
Copy link
Member Author

Avaq commented Jul 19, 2018

After some consideration, I think it needs a 3.0.0. Releasing it as anything else would be like changing the rules during the game. I find that most changes to configuration bundles constitute major version bumps.

@davidchambers
Copy link
Member

Okay. Let's update to [email protected] while we're at it. :)

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

Successfully merging this pull request may close these issues.

2 participants