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

fix: ignore SNOWPACK_PUBLIC_ env variable when generating __snowpack__/env.js #924

Merged
merged 4 commits into from
Aug 27, 2020

Conversation

gr2m
Copy link
Contributor

@gr2m gr2m commented Aug 26, 2020

@gr2m gr2m requested a review from a team as a code owner August 26, 2020 16:50
@vercel
Copy link

vercel bot commented Aug 26, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/4ale66yvb
✅ Preview: https://snowpack-git-900-follow-up-snowpack-public-env-variables-regex.pikapkg.vercel.app

Copy link
Owner

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

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

LGTM!

@gr2m
Copy link
Contributor Author

gr2m commented Aug 26, 2020

I think the error on the Windows builds is legit. I tried to restart the build but it persists:

FAIL test/build/build.test.js (21.49 s)
  ● snowpack build › html-environment-variables

    Command failed with exit code 1: yarn testbuild
    'be' is not recognized as an internal or external command,
    operable program or batch file.
    events.js:174
          throw er; // Unhandled 'error' event
          ^

    Error: spawn be ENOENT

The testscript in package.json is

https://github.com/pikapkg/snowpack/pull/924/checks?check_run_id=1033570802#step:4:133

"cross-env SNOWPACK_PUBLIC_MY_ENV_VAR='my-var-replacement-configured-in-package.json' SNOWPACK_PUBLIC_='should be ignored' snowpack build"

I wonder if the "'be' is not recognized as an internal or external command" comes from the 'should be ignored' ` value? Any idea?

@FredKSchott
Copy link
Owner

ha! that's fascinating.

I don't know enough about cross-env, but i'd say just change it to 'ignoreme' or 'tobeignored' and call it a day :)

@FredKSchott
Copy link
Owner

Still LGTM, feel free to merge whenever you get tests passing :)

@gr2m gr2m merged commit fad13bd into master Aug 27, 2020
@gr2m gr2m deleted the 900/follow-up-snowpack-public-env-variables-regex branch August 27, 2020 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants