Convert project to es modules

[rellafella]

This is unfortunately all in one giant commit, I noticed you already had a PR open for this so if this doesn't get merged I thought at least you might find some of the aspects useful.

My main issues were around getting jest to behave with es modules, from what I could tell generally people will keep the project type as commonjs when using jest, but they will just convert files to esm as they need to.

I have converted the test suite to use vitetest instead.

As far as updates go outside of the usual require and module.exports cleanup I have made some notes below where there have been any significant improvements.

• Moved switch plugin from the tests dir to the src dir to keep the tests clean
• (src/plugins/switch-plugin/index.js) not sure if I have got the exports right here, but it seems to work...
• (src/util/pluginUtil.js) handling the dynamic import for plugins has been changed a bit to handle the Promise resolution in ESM

[zackad]

LGTM. Test failed on prettier version 2 which personally I want to drop the support. Not sure what people think, should we keep compatibility with prettier 2 or drop it?

[rellafella]

I would rather see this move forward than have optimal backwards compatibility to be honest. However I might be able to take a look on the weekend to see what it would take to specify both an index.js and a index.cjs entry point. Pretty sure that would solve the install issue, but not sure that the code would be fully backwards compatible with common js, definitely not a module expert.

[zackad]

However I might be able to take a look on the weekend to see what it would take to specify both an index.js and a index.cjs entry point. Pretty sure that would solve the install issue, but not sure that the code would be fully backwards compatible with common js, definitely not a module expert.

The way I see it, we might need to add build step before publishing into npm registry. Similar to how trivago/melody handling support for commonjs and es module.

OTOH, there's problem with plugin system. Here what the output when loading switch-plugin

CommonJS

{% switch matrixBlock.type %}
    {% case 'text' %}
        {{ matrixBlock.textField|markdown }}
    {% case 'image' %}
        {{ matrixBlock.image[0].getImg() }}
    {% default %}
        <p>
            A font walks into a bar.
        </p>
        <p>
            The bartender says, “Hey, we don’t serve your type in here!”
        </p>
{% endswitch %}

ES Module

{% switch matrixBlock.type %}
{% case 'text' %}
    {{ matrixBlock.textField|markdown }}
{% case 'image' %}
    {{ matrixBlock.image[0].getImg() }}
{% default %}
    <p>
        A font walks into a bar.
    </p>
    <p>
        The bartender says, “Hey, we don’t serve your type in here!”
    </p>
{% endswitch %}

Still can't figure it out what the actual problem are.

[rellafella]

That's a good catch, i'll investigate here, I think this probably has something to do with this export, whilst being valid I don't think it is correct.
https://github.com/zackad/prettier-plugin-twig-melody/blob/bcc1e020c5c0356e7d209e8e2bd7d9ac5faedb2e/src/plugins/switch-plugin/index.js#L33-L35

[zackad]

I think the problem is how plugin being loaded. I can't even make the plugin to be executed. The variable result returning an empty array. Tried async/await function and still no success.

Something like this and other variation doesn't seems to work.

return pluginPaths.map(async pluginPath => await tryLoadPlugin(pluginPath));

[zackad]

I've partially applied your changes into separate commit/pull request. All that left is actually rewrite into es module. Would you be able to rebase?

Some notes:

• revert changes to snapshot test file. rewriting should not modify actual test result.
• revert change on flake.nix, unrelated
• add .eslintcache into .gitignore

Thank you.

[rellafella]

I am going back over this with some proper attention to detail. Do you have a preference on using or avoiding default exports?

edit: actually never mind, I am going to keep this as like-for-like as possible. Anything named will stay named, anything default will stay as default.

[zackad]

I have preference to keep the changes to be as minimal as possible. Unfortunately your changes is not exactly compatible with existing plugin functionality as you can see from modified snapshot file.

Regarding plugin system, I'm not sure if I want to keep this feature. The switch-plugin is not a twig feature AFAIK. Personally I want to remove plugin system. Do people actually use it? What do you think?

I haven't test it on my local machine (hopefully this weekend).

[rellafella]

I am just working on it now actually stand by for some updates to this branch.

I think that is a valid suggestion to just scrap the plugin functionality or at least remove it for now, to perhaps add it in later if it is going to stall further development.

Whilst the switch-plugin isn't part of twig, it is part of Craft CMS (since at least 2.0.0) which is a common use case for this prettier plugin.

Out of curiosity what system are you using Twig for and does that come bundled with some custom Twig extensions that you want to include?

Craft CMS docs for tags

https://craftcms.com/docs/2.x/templating/switch.html
https://craftcms.com/docs/3.x/dev/tags.html
https://craftcms.com/docs/4.x/dev/tags.html
https://craftcms.com/docs/5.x/reference/twig/tags.html

[zackad]

I'm using twig with symfony framework. Didn't know about twig use case outside symfony framework ecosystem. Knowing this, I don't want to break existing functionality (atleast without offering some alternatives).

For now I want to keep all features as-is. Extension/features I want to add in the future includes:

• all the feature from original twig templating engine
• features from twig component

[rellafella]

I would say the easiest way would be to just scrap the plugin utility and move to an approach where that functionality is built into the core

[rellafella]

I have updated the PR with an example of what this would look like without the plugin feature and the switch tag print just rolled directly into the core

[zackad]

Thank you @rellafella