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: Add WEB_OAUTH flag to build the web version but with OAuth Auth #1796

Closed
wants to merge 1 commit into from

Conversation

Crash--
Copy link
Contributor

@Crash-- Crash-- commented Apr 7, 2020

The idea is to be able to embed our web build in an other native app
and load it locally.

run: env WEB_OAUTH=true yarn build:browser

The idea is to be able to embed our web build in an other native app
and load it locally.
@Crash-- Crash-- requested a review from ptbrowne as a code owner April 7, 2020 09:31
})
]

if (WEB_OAUTH) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure that WEB_OAUTH is the right term here since we have a reference to the cozy-bar below, which is not linked to "oauth". Could we find a better term here ? Should we have another target instead of webpack.target.browser ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to create too much noise around the webpack config since you're migrating it to cozy-scripts. I don't know if I really want that PR to be merged right now.

But yeah you're right, I think the goal is to:

  • keep the OAUTH__AUTH flag to know if we want an OAuth auth or not and remove the check for TARGET === mobile
  • add an embedded target
  • add an embedded config

The result will be:

  • target mobile will use the webpack.config.embedded to inject cozy-bar, cozy-client-js but keep other plugins like providing pouchDB stuff
  • target embedded will only use webpack.config.embedded I think
  • Remove the check of target === mobile to use OAUTH_AUTH when needed.

WDYT about that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it !

@Crash--
Copy link
Contributor Author

Crash-- commented Jul 20, 2020

I'm closing this PR since there is no need anymore to use this kind of hack. We use an override to do what we have to do, and we only serve the web version for now.

@Crash-- Crash-- closed this Jul 20, 2020
@ptbrowne ptbrowne deleted the oauth/web branch July 20, 2020 07:34
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