OpenSourceRaidGuild / babel-vite

babel preset and plugins that emulate Vite's non-standard functionality
MIT License
57 stars 12 forks source link

Pr/prefix config option #9

Closed andrew-kramm closed 9 months ago

andrew-kramm commented 2 years ago

What:

Added the ability to configure an alternate prefix when transforming environmental variables (instead of VITE_).

{
  "plugins": [    
    [
      "babel-plugin-transform-vite-meta-env", 
      { "prefix": "REACT_APP_" }
    ]
  ]
}
{
  "plugins": [    
    [
      "babel-plugin-transform-vite-meta-env", 
      { 
        "prefix": ["REACT_APP_" , "VITE_"]
      }
    ]
  ]
}

Why:

I would like to use this plugin with a React app that is being updated to use Vite instead of create-react-app, but I need the ability to inject environmental variables with a different prefix.

How:

I removed VITE_ as a possible Regex matcher and added code to use an alternate prefix from the plugin settings.

Checklist:

Thank you for considering this PR! 🙂

Note: The script npm run contributors:add does not exist so I manually added myself to .all-contributorsrc.

agriffis commented 2 years ago

This would also be a big help in storybook with the STORYBOOK_ prefix, when re-using sources from the main tree in a local storybook addon.

agriffis commented 2 years ago

@andrew-kramm Storybook's Vite builder accepts an array param for this. Would it be hard to amend this PR to do the same? What I'd like is ['VITE_', 'STORYBOOK_']

andrew-kramm commented 2 years ago

@andrew-kramm Storybook's Vite builder accepts an array param for this. Would it be hard to amend this PR to do the same? What I'd like is ['VITE_', 'STORYBOOK_']

Yes, that is an easy update. I will push an update in the next day or so.

andrew-kramm commented 2 years ago

I pushed a commit to support a string or an array of strings.

codecov-commenter commented 2 years ago

Codecov Report

Merging #9 (a4f2dd0) into main (5e04373) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main        #9   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           67        85   +18     
  Branches        22        29    +7     
=========================================
+ Hits            67        85   +18     
Impacted Files Coverage Δ
.../babel-plugin-transform-vite-meta-env/src/index.ts 100.00% <100.00%> (ø)
packages/babel-preset-vite/src/index.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5e04373...a4f2dd0. Read the comment docs.

mpeyper commented 2 years ago

Can you talk a little bit more about the use cases here? This plugin is specifically to replicate Vite functionality, which, as I understand it, is an alternative to CRA.

I get that this is a relatively minor change to the code, but I’m wary to move too far away from its purpose.

agriffis commented 2 years ago

@mpeyper My code ends up running in a few different contexts:

  1. Build for release with Vite.
  2. Run tests in Jest with Babel.
  3. Develop in Storybook which has a Vite builder, but has a different prefix for environment variables
  4. Run storyshot tests combining Storybook with Jest.
  5. Run my code as part of an add-on in the Storybook manager.

The biggest problem is that different projects want to impose different prefixes, and they aren't always configurable, so it's hard to make them agree. This PR would help to ease the integration.

Also I'd point out that Vite itself is also configurable, and like this PR, allows either a string or array of strings: https://vitejs.dev/config/#envprefix

so currently the Babel plugin doesn't properly replicate Vite until this PR is merged. :stuck_out_tongue_winking_eye:

mpeyper commented 2 years ago

Thanks for the explanation @agriffis. The fact that vite is also configurable in the same way means we need this 😅

I’ll give the code a proper review after the kids go to bed tonight, but expect it to be merged soon.

mpeyper commented 2 years ago

I made some changes to the PR, the main ones being changing the option to envPrefix to match the Vite config setting and to allow the options to be set for the preset as well and passed through the plugin via the existing env.

The other minor changes were typing the option for the plugin, and restructuring the README docs to match the style of the Options docs of the preset for consistency.

I also added the missing all-contributors script for the next contributor 😅

While I was going through this, I thought it would be really nice if we could default to the configured envPrefix of the Vite config (falling back to VITE_ if not configured) rather than forcing users to set it twice, but Ii's probably more effort than it's worth. Definitely not required for this PR, but if someone reading this want to have a crack at it in a follow up PR I'll happily review it.

mpeyper commented 2 years ago

I'll let this sit now until tomorrow night to give a chance for feedback on my additions.

andrew-kramm commented 2 years ago

Changes look good. The README.md needs the following updates to reflect the code changes:

const mode = process.env.MODE;
const baseUrl = '/';
const nodeEnv = process.env.NODE_ENV || 'test';
const dev = process.env.NODE_ENV !== 'production';
const prod = process.env.NODE_ENV === 'production';
const viteVar = process.env.VITE_VAR;
const other = {
  ...Object.fromEntries(Object.entries(process.env).filter(([k]) => k.startsWith('VITE_'))),
  NODE_ENV: process.env.NODE_ENV || 'test',
  MODE: process.env.NODE_ENV || 'test',
  BASE_URL: '/',
  DEV: process.env.NODE_ENV !== 'production',
  PROD: process.env.NODE_ENV === 'production'
}.OTHER;
const env = {
  ...Object.fromEntries(Object.entries(process.env).filter(([k]) => k.startsWith('VITE_'))),
  NODE_ENV: process.env.NODE_ENV || 'test',
  MODE: process.env.NODE_ENV || 'test',
  BASE_URL: '/',
  DEV: process.env.NODE_ENV !== 'production',
  PROD: process.env.NODE_ENV === 'production'
};

And perhaps a code example showing multiple prefixes:

With multiple prefixes:

{
  "plugins": [
    [
      "babel-plugin-transform-vite-meta-env",
      {
        "envPrefix": ["CUSTOM1_", "CUSTOM2_"]
      }
    ]
  ]
}

Also, consider using the js language for the code blocks in the "In" and "Out" sections.

andrew-kramm commented 2 years ago

@mpeyper Just checking in. How is the PR coming?

mpeyper commented 2 years ago

I’ve not looked at the documentation improvements (very busy week), but agree with the intention. Unfortunately I’m away for the weekend and won’t be able to look at anything until Monday now.

If you want to add code snippets to the Options sections and fix the output examples then I can review and merge from here.