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

Create PWA tool #1198

Closed
wants to merge 24 commits into from
Closed

Create PWA tool #1198

wants to merge 24 commits into from

Conversation

zetlen
Copy link
Contributor

@zetlen zetlen commented May 3, 2019

Description

Refactor Buildpack as a CLI utility and add a npm init @magento/pwa starter kit.

Additions

Buildpack CLI

$ buildpack --help
buildpack <command>

Commands:
  buildpack create-env-file <directory>     Generate a .env file in the provided
                                            directory to store project
                                            configuration as environment
                                            variables
  buildpack init-custom-origin <directory>  Get or create a secure, unique
                                            hostname/port combination and a
                                            trusted SSL certificate for local
                                            development.
  buildpack init-project <template>         Create a PWA project in <directory>
  <directory>                               based on <template>.
  buildpack validate-env <directory>        Check the current environment,
                                            including .env file if present, to
                                            ensure all required values are
                                            present.

Options:
  --version  Show version number                                       [boolean]
  --help     Show help                                                 [boolean]

The big feature is the subcommand init-project, which through a combination of arguments and flags will create a new project directory.

$ buildpack init-project --help
buildpack init-project <template> <directory>

Create a PWA project in <directory> based on <template>.

Positionals:
  template   Name of a "template" to clone and customize. Currently the
             "venia-starter" template is supported: `buildpack init-project
             venia-starter <directory>     [required] [choices: "venia-starter"]
  directory  Name or path to a directory to create and fill with the project
             files. This directory will be the project root. [string] [required]

Project configuration:
  --backendUrl, -b  URL of the Magento 2.3 instance to use as a backend. Will be
                    added to `.env` file.
  --customOrigin    Create a custom secure host and certificate for this
                    project. Required for full PWA functionality. Requires
                    administrator privileges.          [boolean] [default: true]

Metadata:
  --name, -n    Short name of the project to put in the package.json "name"
                field. Uses <directory> by default.
  --author, -a  Name and (optionally <email address>) of the author to put in
                the package.json "author" field.

Package management:
  --install    Install package dependencies after creating project
                                                       [boolean] [default: true]
  --npmClient  NPM package management client to use.
                                       [choices: "npm", "yarn"] [default: "npm"]

Options:
  --version  Show version number                                       [boolean]
  --help     Show help                                                 [boolean]
  • TODO: Add more subcommands to simpify scripts
  • TODO: Tests and docs

This command is non-interactive. The friendly facade on top of it is the create-pwa package.

npm init @magento/pwa

NPM initializer, like create-react-app.

image

Could be expanded to other project templates.

Changes

  • Organize configuration of all tools to use a common interface: environment variables and env-to-option translators
  • Refactors of Venia Adapter, router, and drivers to make the whole app portable enough to use.

Screenshots / Screen Captures (if appropriate):

Proposed Labels for Change Type/Package

  • major (e.g x.0.0 - a breaking change)
  • minor (e.g 0.x.0 - a backwards compatible addition)
  • patch (e.g 0.0.x - a bug fix)

This is Buildpack 3.0.0.

Checklist:

  • I have updated the documentation accordingly, if necessary.
  • I have added tests to cover my changes, if necessary.

(Rest of PR TBD.)

@vercel
Copy link

vercel bot commented May 3, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://venia-git-zetlen-create-pwa-cli.magento-research1.now.sh

@zetlen
Copy link
Contributor Author

zetlen commented May 3, 2019

DO NOT MERGE, there is still some internal plumbing especially in the package files :)

Just wanted to get some eyes on it.

@zetlen
Copy link
Contributor Author

zetlen commented May 3, 2019

This PR's merge-base is actually #1078 so the changeset will look really large until #1078 is merged.

@zetlen zetlen changed the base branch from develop to jimbo/searchbar May 3, 2019 20:04
@zetlen zetlen changed the base branch from jimbo/searchbar to develop May 7, 2019 18:29
@zetlen zetlen mentioned this pull request May 7, 2019
2 tasks
@zetlen zetlen changed the base branch from develop to zetlen/publish-compat-manifest-with-gqlcli-1214 May 7, 2019 19:00
@zetlen zetlen changed the base branch from zetlen/publish-compat-manifest-with-gqlcli-1214 to zetlen/remove-unused-buildpack-tools May 7, 2019 19:08
@zetlen zetlen changed the base branch from zetlen/remove-unused-buildpack-tools to zetlen/refactor-router-drivers-for-portability May 7, 2019 19:24
@zetlen zetlen force-pushed the zetlen/refactor-router-drivers-for-portability branch from e6d1d43 to 917853b Compare May 7, 2019 19:27
@zetlen zetlen force-pushed the zetlen/refactor-router-drivers-for-portability branch from 917853b to d309335 Compare May 7, 2019 20:06
@zetlen zetlen changed the base branch from zetlen/refactor-router-drivers-for-portability to zetlen/graphql-playground-autodetects-queries May 7, 2019 21:31
@zetlen zetlen changed the base branch from zetlen/graphql-playground-autodetects-queries to zetlen/unified-config May 8, 2019 13:54
@brendanfalkowski
Copy link
Contributor

brendanfalkowski commented May 31, 2019

@zetlen — I think there's an issue with .gitignore that is copied from /packages/venia-concept/.gitignore into the newly created project as root /.gitignore.

That file looks like this:

.idea{/*,/**/*}
.vscode{/*,/**/*}
coverage{/*,/**/*}
node_modules{/*,/**/*}
storybook-dist{/*,/**/*}
test-results{/*,/**/*}
dist{/*,/**/*}
.DS_Store
.env
build-stats.json
npm-debug.log
lastCachedGraphQLSchema.json
test-report.xml
test-results.json
yarn-error.log

Git doesn't recognize the globbing syntax {/*,/**/*} so it wants to track files that should be ignored. I couldn't find any info on Git supporting a curly-brace syntax, so I'm guessing this was meant to be rewritten during buildpack creation.

PWA Studio repo's root /.gitignore doesn't have globs or this curly-brace syntax, so those folder names match the underlying dirs in /packages/venia-concept/ and they're ignored.

node_modules
npm-debug.log
.DS_Store
.vscode
jsconfig.json
coverage
test-results
dist
storybook-dist
.idea
test-report.xml
test-results.json
lerna-debug.log
.env
yarn-error.log
# Packages that build partially transpiled ES modules put them here
packages/*/esm/*
docker/certs

When running npm init @magento/pwa then Git tries to track everything:

Screen Shot 2019-05-31 at 4 26 16 PM

I'm not sure if that's intentional, but I wasn't expecting to see dependencies + generated files tracked when using the buildpack create tool. When I removed the curly-brace globs it drops the trackable files from 45,000 to 50.

@zetlen zetlen closed this Jun 3, 2019
@brendanfalkowski
Copy link
Contributor

@zetlen — So did the ticket closing mean...

  1. How dare you!
  2. We already fixed this elsewhere.
  3. Create a new ticket and send PR please.
  4. Won't fix.
  5. Really, you don't know that undocumented Git syntax? Hey everyone, come look at this guy!

I can do option 3.

@zetlen
Copy link
Contributor Author

zetlen commented Jun 27, 2019

Ack, @brendanfalkowski I can't believe I missed these. I didn't mean to close it as a response to your comment; I didn't even see your comment! I closed it as bookkeeping because it's now a long-running branch with a mind of its own, and it needs to be split up into separate PRs a second time. 💯

The bad gitignores are definitely not making their way into the final PR. Thanks for noticing them.

@brendanfalkowski
Copy link
Contributor

No worries

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.

3 participants