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

feat: jsx migration script #869

Merged
merged 19 commits into from
Aug 29, 2024
Merged

feat: jsx migration script #869

merged 19 commits into from
Aug 29, 2024

Conversation

KaiVandivier
Copy link
Contributor

@KaiVandivier KaiVandivier commented Aug 7, 2024

LIBS-657

The script:

  1. Scans .js files in /src for JSX syntax, then renames them to .jsx
    1. Question: do any other dirs need scanning? I added the option to add a custom glob string (not exactly a normal shell glob)
    2. .ts files aren't handled; it's assumed that TS enforces using .tsx extensions for files with JSX syntax... although it would be easy to add
  2. Updates import paths to renamed files in src
  3. Updates entryPoints values in d2.config.js, if needed

I started this trying to use regexes to detect JSX syntax and import statements, but it ended up being best to use an AST parser. It's a bit slower, but still only took a few seconds for the thousands of JS files in the capture app, for example

Looking for input:

  1. Is this the right place/repo for this script?
  2. Is there a better name for it than jsx-migration?

Recommended testing

You can test this out in an app (or more) of your choice -- here are a few that I've tried:

  1. Data Visualizer -- has exports
  2. Scheduler -- uses imports without file extensions; can test out the skipUpdatingImportsWithoutExtension flag. Somewhere there's a file that has two imports from the same file (separated to group the objects conceptually) -- this is a test of the import replacement logic
  3. Capture -- uses Flow & loads of files

Steps:

  1. Checkout this branch in app-platform
  2. In your app(s) of choice, install the @dhis2/cli-app-scripts alpha & deduplicate app-runtime: yarn add @dhis2/cli-app-scripts@alpha && npx yarn-deduplicate yarn.lock && yarn
    3. Watch out for any other resolutions on relevant libs: cli-app-scripts, app-runtime
  3. Since a lot of files may be changed, it's nice to take advantage of version control to undo this step if it doesn't go as intended. In VSCode, I like to stage files that I want to keep around like package.json and yarn.lock before running the script. Then, if I want to undo the results of the script, I can click the "Discard All Changes" undo arrow in the Source Control tab on the Unstaged Changes section and go back to the beginning (see video below)
  4. Use a path to the CLI bin to run the local version of the script, e.g. ../app-platform/cli/bin/d2-app-scripts jsx-migration
  5. You can test out some flags:
    7. --debug to get detailed logs
    8. --globString "..." to test out a custom glob (the default is src/**/*.js)
    9. --skipUpdatingImportsWithoutExtension -- maybe self-explanatory (I figure some repos will want to leave the imports without file extensions. Functionally, this works fine with Vite. In some repos though, this creates linting complaints, but I'm sure the linting rules can be updated)
  6. Run yarn start to make sure the app works

VSCode Source control:

source-control.mov

@KaiVandivier KaiVandivier marked this pull request as ready for review August 15, 2024 12:50
@amcgee
Copy link
Member

amcgee commented Aug 15, 2024

What do you think about creating a new codemod command namespace? So something like....

d2-app-scripts migrate <name-of-migration>
d2-app-scripts migrate js-to-jsx

@kabaros kabaros requested a review from a team August 27, 2024 11:33
Copy link
Contributor

@kabaros kabaros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 tested this on few apps (login, data-entry and app-management) and it works as expected. I wonder if the docs here are a good place to document the caveats (i.e. failure for some import *...) and any other things that might need to be checked manually.

@KaiVandivier
Copy link
Contributor Author

Thanks for the review!

The import * from ... quirk is related to Vite's module resolution I think, rather than the filename migration -- since that's a node module, it'd be out of the scope this script, which is just renaming files and updating local imports

That said, it'll definitely be documented in the larger Vite migration documentation 👍

@KaiVandivier KaiVandivier merged commit 7764f49 into alpha Aug 29, 2024
6 checks passed
@KaiVandivier KaiVandivier deleted the jsx-migration-script branch August 29, 2024 14:35
dhis2-bot added a commit that referenced this pull request Aug 29, 2024
# [12.0.0-alpha.11](v12.0.0-alpha.10...v12.0.0-alpha.11) (2024-08-29)

### Features

* jsx migration script ([#869](#869)) ([7764f49](7764f49))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 12.0.0-alpha.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants