Fix broken tree shaking for legacy parse

[marcodejongh]

Fixes #14

[biniek-io]

Thanks

[marcodejongh]

@dkozickis @kossnocorp @silvenon Could you do us a huge favour and review and merge this? It's a relatively low-risk change that has a big impact, without this change we're not able to move forward with our date-fns upgrade, unless we remove date-fns/upgrade as a dependnecy.

[marcodejongh]

Needed to fix tsconfig module resolution, wonder if that is why the old style import was used to begin with. We're now using this version in prod published as date-fns-upgrade-treeshaking@1.0.8, so should be good to go now

[silvenon]

Could you help reviewers understand what the problem is and how these changes fixes it? I think it would greatly increase the chances of merging.

[marcodejongh]

import { isDate } from 'date-fns'

Imports the whole module and doesn't get tree-shaken by Webpack.
By changing this to:

import isDate from 'date-fns/isDate'

There is no more need to tree-shaken because we're now importing from a single dedicated file.

The changes to the config were necessary to facilitate those changes as TS is a bit fickle in its module resolution logic. date-fns exports isDate on the module.exports it loses its "default" key, by enabling esModuleInterop, TS protects against this scenario, resulting in the code working.

[silvenon]

Adding an ES build of this library seems like a more natural solution for this problem, that way people won't end up with two copies of isDate (date-fns/isDate and date-fns/esm/isDate).

[marcodejongh]

@silvenon Not sure I understand? date-fns currently doesn't have an ESM build: https://unpkg.com/browse/date-fns@2.21.1/isDate/
The recommended method of importing date-fns: https://date-fns.org/docs/Getting-Started#submodules

You're right that if:

import { isDate } from 'date-fns'

Was ESM, we would need to do this change, but the fact that date-fns didn't add an ESM build up until now, leads me to believe this is an intentional decision. Arguably, we'd still want to import by submodules as this is still more reliable than tree shaking.

[silvenon]

I don't know how to get to it with unpkg's browse feature, but date-fns/esm/isDate/index.js appears to exist, but now I realized that date-fns/esm/isDate/package.json points ESM-enabled tools to date-fns/esm/isDate/index.js anyway, so there's no problem, my bad! 😃

[silvenon]

Only now I realized I don't have merging rights. 🤦‍♂️ 🤦‍♂️ 🤦‍♂️ @dkozickis

[marcodejongh]

Ah it does have ESM, missed that...

I just noticed that date-fns/upgrade didn't tree shake properly when bundle analysing, I'm not quite sure why it doesn't tree shake https://unpkg.com/date-fns@2.21.1/esm/index.js looks fine, but Webpack can be a bit fickle sometimes, it's better to just use the submodule import instead.

[silvenon]

Because @date-fns/upgrade only has a CJS build, so that's where the tree-shaking potential stops. 😕 You'll need to edit Makefile to produce two builds. To create an ESM build you can probably just pass flags to tsc, and make sure to add module field to package.json. Let me know if you need additional help.

[marcodejongh]

Adding ESM sounds like a good follow-up task on this project, but has significantly bigger scope and as a result a higher risk profile.
This PR is significantly less risky and fixes the more immediate problem of pulling in all of date-fns when trying to use this upgrade module.