Use let instead of const

[ekilah]

Fixes #97

This avoids errors in the Webstorm IDE's TypeScript console.

[ekilah]

I guess I should clarify, it seemed to resolve things, but maybe the CI failure means that it was just broken in a different way? Unclear. I'll let you tell me, I am just observing symptoms :)

[rschristian]

The tests are only failing because you didn't update the snapshots.

Did you not try to run the tests locally or read the output from the CI?

[ekilah]

I did nothing like that no. I don't see any instructions on the repo's markdown files describing how to do what you are implying is necessary. Let me know and I can try those things.

[rschristian]

This is the output from the CI. It's a snapshot test. It's saying it expected const but found let in it's place.

FAIL src/helpers/__tests__/getDtsSnapshot.test.ts (8.839s)
  ● utils / cssSnapshots › with file 'test.module.css' › createExports › should create an exports file

    expect(received).toMatchSnapshot()

    Snapshot name: `utils / cssSnapshots with file 'test.module.css' createExports should create an exports file 1`

    - Snapshot  - 1
    + Received  + 1

    @@ -1,6 +1,6 @@
    - declare const classes: {
    + declare let classes: {
      'classA': string;
        'ClassB': string;
        'class-c': string;
        'class_d': string;
        'parent': string;


      72 |             options: {},
      73 |           });
    > 74 |           expect(exports).toMatchSnapshot();
         |                           ^
      75 |         });
      76 |       });
      77 |

Running tests locally and/or reading CI output is kinda important.

You'll need to update the snapshots.

[ekilah]

So is having clear expectations and instructions on a project that anticipates public input from people unfamiliar with the codebase. Not getting a reply in 3 weeks isn't a good incentive to go diving into the various test suites to fix CI either.

But I'm not here to complain, I'm just trying to report an issue and help solve it. I don't think the subtle condescension about the importance of doing this and that are productive or earned.

If you don't mind posting the set of commands necessary to do what you're asking for, I can do that next time I have some time. You neglected to include instructions.

[rschristian]

Firstly, I'm not connected with this project in any way, shape, or form. Just came across it tonight.

Not getting a reply in 3 weeks isn't a good incentive to go diving into the various test suites to fix CI either.

A good incentive is the failing test suite and CI though. The CI failed in less than a minute.

I don't think the subtle condescension about the importance of doing this and that are productive or earned.

You didn't even try to run the tests locally or even glance at the CI output to see why the tests were failing. I'm not exactly sure what you expect?

If you don't mind posting the set of commands necessary to do what you're asking for

There are no magic commands. Completely resetting the snapshots is almost never recommended (especially not from a PR) as that opens up the possibility for incorrect snapshots. You should fix them by using a search & replace or by hand. It's just swapping out const with let.

[ekilah]

Thanks for your valuable insight.

[mrmckeb]

Sorry @ekilah, I've been away and had a few personal things to attend to, and I didn't get back to this as soon as I wanted to.

As @ryanchristian4427 said, the snapshots are currently failing here, which need to be fixed. If you can get those fixed, I can help get this merged and released. As the tests are in Jest, you can simply run the command and pass -u to update snapshots, or press u if in interactive mode. You can read more about snapshot testing in Jest here.

Have you tested these changes? If so, can you please provide a before and after screenshot too? I don't have Webstorm installed (I use VSCode).

[ekilah]

Thanks for getting back to me!

I rebased to get the TS 4.0 fix in #101, and updated the snapshots with yarn test -u.

Without my patch:

With my patch:

Note that this just prevents the spammy errors in WebStorm; as far as I can tell, this plugin doesn't add any functionality in WebStorm. Without this plugin, WebStorm can already do this:

That last statement somewhat conflicts with #96 - the OP seems to get some use out of this plugin that I'm not seeing, but at least this PR fixes the errors.

[mrmckeb]

Thanks for that - so to clarify, this fix is because the plugin is in your project for those in your team using VSCode, but you don't need it.

On a side note, VSCode also has a few extensions that can do this, but obviously each user needs to install and configure those themselves.

[mrmckeb]

Hi @ekilah, I've published this as 2.6.0-beta.0. It's built with TypeScript 4.0, and also contains a minor change to make the classnames readonly (as they should be).

Please let me know if this a) fixes your problem, and b) causes any issues for you or colleagues.

[ekilah]

replied over at #97 (comment)