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

Use string literal names rule #117

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Dschungelabenteuer
Copy link
Member

@Dschungelabenteuer Dschungelabenteuer commented Jan 17, 2023

Issue: #111

What Changed

  • Added ESLint rule to enforce string literals to override Story names.
  • This supports both the following patterns:
export const MyStory = {, name}
const MyStory = {, name}
export { MyStory }

Documentation and any plain english content may need to be rephrased.

Checklist

Check the ones applicable to your change:

  • Ran yarn update-all
  • Tests are updated
  • Documentation is updated

Change Type

Indicate the type of change your pull request is:

  • maintenance
  • documentation
  • patch
  • minor
  • major

@yannbf yannbf added the minor Increment the minor version when merged label Jan 25, 2023
@yannbf
Copy link
Member

yannbf commented Jan 25, 2023

This is fantastic @Dschungelabenteuer ! I made a few tweaks and added a few more test cases which we need to support. Can you take a look?

@Dschungelabenteuer
Copy link
Member Author

Dschungelabenteuer commented Jan 25, 2023

@yannbf It looks good to me, should I go green? I'll work something out :)

@Dschungelabenteuer
Copy link
Member Author

Sorry for being that long to address these cases @yannbf, I've been a bit busy lately!

While taking a look at some other issues (including #28 and #67 you mentioned on Discord), I ended up being a bit more ambitious than just addressing the original issue and its test cases.

I definitely wanted to have a generic, easy and re-usable way of restricting validation logic to stories and their properties. I came up with some wrapper function which abstracts the whole process of "figuring out whether we're in a Story context" and exposes a very simple API to get stories and story properties. You can even filter them, which can come handy as illustrated by the use-string-literal-names rule, which specifically targets both name and storyName properties. A few notes:

  • It should work with the test cases you provided (including the "spread" scenario)
  • Code mentions "properties" instead of "parameters" so that it does not create any confusion with the actual parameters property of a Story.

I believe this could make writing new rules much easier, and bring in a bit of confidence about what's going on under the hood. The rule I've wrote to address the original issue now just holds in <50 LOC! However, I could not come up with any good approach on how to unit test the wrapper, which kind of upsets me!

I'd love to get your insight!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants