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

Enable use with Vite #1402

Merged
merged 4 commits into from Oct 7, 2021
Merged

Enable use with Vite #1402

merged 4 commits into from Oct 7, 2021

Conversation

lee-orr
Copy link
Contributor

@lee-orr lee-orr commented Aug 17, 2021

Adjust the way openpgp accesses the NODE_ENV, so that it works in both webpack and vite. should close: #1332

@twiss
Copy link
Member

twiss commented Aug 17, 2021

If at all possible, I would prefer not to add a workaround for this in OpenPGP.js. It seems like it should already be fixed in Vite? (See vitejs/vite#3801 (comment)).

@lee-orr
Copy link
Contributor Author

lee-orr commented Aug 17, 2021

I'm going to close the PR - it seems like there is another solution. It still doesn't work when calling import * as openpgp from "openpgp";, but it works when calling import * as openpgp from "openpgp/dist/openpgp";. Not sure why.

@lee-orr lee-orr closed this Aug 17, 2021
@lee-orr lee-orr reopened this Sep 15, 2021
@lee-orr
Copy link
Contributor Author

lee-orr commented Sep 20, 2021

Re-opened this following the discussion here: #1332

@lee-orr
Copy link
Contributor Author

lee-orr commented Oct 5, 2021

@twiss Can we please bring this in? It's a minor change that will not affect current behavior with webpack at all, but will enable this to work properly in Vite. I did try to fix the issue on the Vite side, but haven't been able to do so, and don't know if/when they will fix it at all.
(FYI - part of the reason I'm asking for this now is that this is for a client project that is ending soon, and they won't be able to have me maintain a fork of openpgp for them - and ideally, they'd just be able to use the actual library, rather than needing to maintain a fork for such a minor difference)

@twiss
Copy link
Member

twiss commented Oct 7, 2021

Hey 👋 Sorry for the delay. I'm not a huge fan of this fix - I would prefer to move in the opposite direction, and make the substitution work correctly. One way I can think of would be to do something like:

const debugMode = (() => {
  try {
    return process.env.NODE_ENV === 'development';
  } catch (e) {}
  return false;
})();

Let me know if that works for you.

@lee-orr
Copy link
Contributor Author

lee-orr commented Oct 7, 2021

Made the adjustment and checked it with my project - looks like it works.

@twiss twiss merged commit df7e690 into openpgpjs:master Oct 7, 2021
@twiss
Copy link
Member

twiss commented Oct 7, 2021

👍 Thanks!

@akiletour
Copy link

Thanks a lot to both of you guys

I will finally be able to migrate from CRA to Vite :)

<3

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.

Cannot import library with Vite
3 participants