Remove MobX-State-Tree flag for non-demo projects #2647

Jpoliachik · 2024-02-21T17:30:39Z

This PR adds support for removing MobX-State-Tree code from a non-demo project.
Retargeted to v10 branch.

Changes:

// @mst inline comments, similar to how demo code removal works
Refactored demo.ts removal logic into more generic markup.ts to be shared with @mst comment types too, and make it easy to add additional other types in the future too.
Added new replace-next-line markup command type, to help swap out lines of code easily without hot patching in scripts (keeps code replacements inline next to other code)

The option shows up --removeDemo=true is set

src/commands/new.ts

src/commands/remove-mst.ts

frankcalise

First pass, it's taking shape nice work!

src/commands/new.ts

src/commands/remove-mst.ts

frankcalise · 2024-02-21T23:24:40Z

src/commands/remove-mst.ts

+    // TODO: full dryRun support
+
+    p()
+    p(`Removing MobX-State-Tree code from '${TARGET_DIR}'${dryRun ? " (dry run)" : ""}`)


What about a try/catch here and if running into any issues, let's warn the user the automated removal could not be completed and refer them to the recipe to do so manually?

A suggestion: if we want to refer people to document officially, I think that document should be docs in Ignite, and not a cookbook recipe.

I added a link to the recipe in the existing try/catch as part of the new flow 👍
I think since it's a tutorial and intended as a backup if this fails, it makes sense to keep it in Cookbook, IMO.

The main reason I say documentation and not a cookbook recipe was that the original intention of cookbook was it to have less obligation to keep it perfect and/or up-to-date (and therefore encourage contributions).

Linking to things officially from here makes it seem more like documentation, to me, which also implies it should continue to be tested with Ignite updates and stay accurate. Drawing a clear line on what we want to keep up-to-date and what's "this worked at one point" seems valuable to me.

Medium GAFO on this, definitely open to disagreement, but that's the thinking behind it.

I'm going to leave as a Cookbook link for now for simplicity, but if we find that the cookbook recipe falls out of date or if we want to integrate the MobX Removal instructions into official Ignite docs in the future, we can make those changes separately

src/commands/new.ts

* feat: cng ftw * fix(babel-config): simplify plugins with SDK 50 * fix(boilerplate): remove unused eslint pkgs * fix(cli): reword workflow question * fix(cli): remove unnecessary warning

frankcalise · 2024-03-28T19:33:33Z

@Jpoliachik can you target this against v10 as the base branch?

* feat(boilerplate): mmkv in over async storage * fix(cli): remove async storage new arch specifics * test(boilerplate): mmkv clean up * fix(boilerplate): storage.load with just a string * test(boilerplate): storage test types

Jpoliachik · 2024-04-05T20:18:19Z

@frankcalise sure thing, I'll work on this early next week 👍

# Conflicts: # boilerplate/app/devtools/ReactotronConfig.ts # src/commands/new.ts

Jpoliachik · 2024-05-09T15:37:29Z

boilerplate/app/models/helpers/useStores.ts

      // reactotron integration with the MST root store (DEV only)
      if (__DEV__) {
+        // @ts-ignore
        console.tron.trackMstNode(rootStore)


not sure why this wasn't needed before, but I was getting typescript errors without.

Jpoliachik · 2024-05-09T15:47:40Z

src/tools/markup.ts

+              result = sanitize(result)
+            } else {
+              result = updateFile(result, markupPrefix)
+              result = sanitize(result)


I don't know if this extra sanitize() was happening before - what behavior do we want here, I assumed we'd always want to get rid of all the markup regardless

# Conflicts: # boilerplate/app/devtools/ReactotronConfig.ts # boilerplate/package.json # src/commands/new.ts

docs/cli/Ignite-CLI.md

lindboe · 2024-07-30T21:24:35Z

src/commands/help.ts

+    )
+    p()
+    command(
+      "remove-mst-markup (rmstm)",


From what I can tell, removal of MST, and removal of MST markup can only be done when generating a new project, since either way the markup won't exist after generation, right?

If that's the case, should we consider these commands "internal" and not document them here, since they can't be used by end users? Or at least add that information in to the docs for context.

Yeah good point - I was copying the pattern for removeDemo, but I suppose both cases wouldn't ever be usable outside of the new flow.

I'll remove the mst & mst-markup help commands - can you think of any reason to keep remove-demo and remove-demo-markup still? (maybe out of scope)

I can't think of any reason, no, @frankcalise ?

I think the reasoning was if you had an existing project where the markup wasn't removed (for the demo markup that is). We didn't always remove the markup out of the box.

That was probably around v8 though and fixed in or around some patch of 9 - we could probably look to remove it as never leaving mark up is what's current.

If we want to address the demo one as a separate PR that's fine

lindboe · 2024-07-30T21:25:45Z

src/commands/remove-mst.ts

+    // TODO: full dryRun support
+
+    p()
+    p(`Removing MobX-State-Tree code from '${TARGET_DIR}'${dryRun ? " (dry run)" : ""}`)


A suggestion: if we want to refer people to document officially, I think that document should be docs in Ignite, and not a cookbook recipe.

boilerplate/app/app.tsx

test/vanilla/__snapshots__/ignite-remove-demo.test.ts.snap

boilerplate/app/app.tsx

Jpoliachik · 2024-08-01T15:54:05Z

This is ready for additional review.

As far as CLI documentation, I added the --mst flag option to the list, and added a blurb under the MST page. I didn't think much else was necessary, since this flag should be straightforward for users, but i'm open to opinions if we think other docs are needed!

lindboe

LGTM, just a small conflict now after the v10 branch has been updated

Thanks for this! 🌟 🥳

# Conflicts: # boilerplate/app/app.tsx

@frankcalise

# [10.0.0](v9.10.1...v10.0.0) (2024-10-14) ### Bug Fixes * **boilerplate:** expo-router initialize i18n ([#2791](#2791) by [@frankcalise](https://github.com/frankcalise)) ([a00113e](a00113e)) * **boilerplate:** Make icon default color explicit, and respond to dark mode ([#2795](#2795) by [@lindboe](https://github.com/lindboe)) ([b82398d](b82398d)) * **cli:** improper MST removal when using CLI prompt, new arch compat ([#2785](#2785) by [@frankcalise](https://github.com/frankcalise)) ([3956652](3956652)) * remove snackify command and references ([#2763](#2763) by [@frankcalise](https://github.com/frankcalise)) ([09671e8](09671e8)) ### Features * Ignite X with Expo 51 ([005ef05](005ef05)) * **boilerplate:** Toggle component refactored to Checkbox, Radio and Switch ([#2667](#2667) by [@frankcalise](https://github.com/frankcalise)) ([54f5208](54f5208)) * **cli:** expo-router option ([#2727](#2727) by [@frankcalise](https://github.com/frankcalise)) ([8bea1f9](8bea1f9)), closes [#2731](#2731) * **cli:** generator options --dir and --case ([#2726](#2726) by [@frankcalise](https://github.com/frankcalise)) ([dff3d24](dff3d24)) * **Theming:** Adds theming support with documentation. ([#2636](#2636)) ([a1ae047](a1ae047)) * **v10:** cng default ([#2641](#2641)) ([c7d998d](c7d998d)) * **v10:** Remove MobX-State-Tree flag for non-demo projects ([#2647](#2647) by @Jpoliachik) ([a738243](a738243)) * add MMKV for v10 ([#2660](#2660) by [@frankcalise](https://github.com/frankcalise)) ([9164e5e](9164e5e)) ### BREAKING CHANGES * Ignite X is out now! This update adds: * Expo Prebuild as default * Dark mode support * Static routing with Expo Router * Flexible generator directory support * Option to remove mobx-state-tree * AsyncStorage->MMKV * New i18n configuration with react-i18next * Lint configuration updates * Toggle component → Checkbox, Radio, and Switch * Improved keyboard avoidance * Component test example configuration Read all about it in our blog article! https://shift.infinite.red/what-is-ignite-x-9ecde0b34d75

infinitered-circleci · 2024-10-14T21:08:10Z

🎉 This PR is included in version 10.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Jpoliachik added 2 commits February 20, 2024 15:22

remove-mst command

efc5983

remove-mst working without code blocks

36e3eb5

jamonholmgren reviewed Feb 21, 2024

View reviewed changes

src/commands/new.ts Outdated Show resolved Hide resolved

jamonholmgren reviewed Feb 21, 2024

View reviewed changes

src/commands/remove-mst.ts Outdated Show resolved Hide resolved

frankcalise reviewed Feb 21, 2024

View reviewed changes

Jpoliachik and others added 10 commits February 22, 2024 17:35

add @mst comments and copied demo remove code

685683e

remove-mst update to include comment removal

ddc4f48

add observer block replacement regex

d2a20e6

mst observer block regex fixes

5b802e0

remove jscodeshift

358cceb

cleanup

a9461a2

alias change

16aef3f

remove types

a508586

generic markup WIP

0c1a5fd

feat(v10): cng default (#2641)

fd9d392

* feat: cng ftw * fix(babel-config): simplify plugins with SDK 50 * fix(boilerplate): remove unused eslint pkgs * fix(cli): reword workflow question * fix(cli): remove unnecessary warning

feat: add MMKV for v10 (#2660 by @frankcalise)

a5bad98

* feat(boilerplate): mmkv in over async storage * fix(cli): remove async storage new arch specifics * test(boilerplate): mmkv clean up * fix(boilerplate): storage.load with just a string * test(boilerplate): storage test types

frankcalise added this to the Ignite v10 milestone Apr 2, 2024

Jpoliachik added 11 commits April 10, 2024 17:12

Merge branch 'v10' into feat/mst-flag

62525e8

# Conflicts: # boilerplate/app/devtools/ReactotronConfig.ts # src/commands/new.ts

fix ReactotronConfig after merge conflict - add back mmkv

2510d33

update new to use --mst flag instead of --removeMST

32c4060

consolidate demo and mst logic into single markup file

a6330e7

includeMST new logic

3697852

remove manual patching from remove-mst

5549b45

start updating tests, use markup.test.ts

8e0f536

update markup tests

488fca5

linting fixes

3615d8d

remove-mst-markup when mst is not removed

055519e

cleanup hardcoded regex

4b004f5

fix 'new' logic for includeMST scenarios

46010ac

Jpoliachik marked this pull request as ready for review May 9, 2024 15:30

Jpoliachik changed the title ~~DRAFT: Remove MobX-State-Tree flag for non-demo projects~~ Remove MobX-State-Tree flag for non-demo projects May 9, 2024

Jpoliachik changed the base branch from master to v10 May 9, 2024 15:35

Jpoliachik commented May 9, 2024

View reviewed changes

update docs and help command

be5ceda

Jpoliachik commented May 9, 2024

View reviewed changes

comment cleanup

fa50198

frankcalise requested a review from joshuayoes May 9, 2024 21:01

frankcalise force-pushed the v10 branch from 0214b07 to 54f5208 Compare May 29, 2024 20:38

Merge branch 'v10' into feat/mst-flag

089cca4

# Conflicts: # boilerplate/app/devtools/ReactotronConfig.ts # boilerplate/package.json # src/commands/new.ts

lindboe reviewed Jul 30, 2024

View reviewed changes

Jpoliachik added 7 commits July 31, 2024 15:01

add documentation about mst usage only if removeDemo

4aa8da0

remove internal remove-mst & remove-mst-markup commands from help menu

28f798a

remove unnecessary comment

135bff6

remove comment

71e2137

add test and snapshot for remove-mst

df3e990

run prettier after mst removal

039e0ac

added remove mst documentation to mst page

46de522

lindboe approved these changes Aug 2, 2024

View reviewed changes

Jpoliachik added 4 commits August 5, 2024 15:31

add cookbook link if mst removal fails

a75fcc8

Merge branch 'v10' into feat/mst-flag

fa726fe

# Conflicts: # boilerplate/app/app.tsx

fix after merge conflict

50d7871

syntax fix

a11b589

Jpoliachik merged commit a738243 into v10 Aug 5, 2024

Jpoliachik deleted the feat/mst-flag branch August 5, 2024 20:01

infinitered-circleci added the released label Oct 14, 2024

Remove MobX-State-Tree flag for non-demo projects #2647

Remove MobX-State-Tree flag for non-demo projects #2647

Uh oh!

Conversation

Jpoliachik commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

frankcalise left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lindboe Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

frankcalise commented Mar 28, 2024

Uh oh!

Jpoliachik commented Apr 5, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jpoliachik commented Aug 1, 2024

Uh oh!

lindboe left a comment

Choose a reason for hiding this comment

Uh oh!

infinitered-circleci commented Oct 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Jpoliachik commented Feb 21, 2024 •

edited

Loading

lindboe Aug 1, 2024 •

edited

Loading