[Doc] Targetables #2966

jcalcaben · 2021-01-22T16:04:49Z

Description

Create documentation for Targetables, this includes:

A brief explainer
An overview for how to use it
Reference documentation with code examples

Related Issue

Closes PWA-796

Acceptance

Any core developer

Verification Stakeholders

Any core developer

Specification

Verification Steps

Navigate to the pwa-devdocs project: cd pwa-devdocs
Run the HTML preview server: yarn develop
Verify site builds
Navigate to /pwa-buildpack/extensibility-framework/
Verify Targetables section exists on the page with accurate information
Navigate to /tutorials/targetables/
Verify page exists, contains accurate information and listed on the left navigation
Navigate to /pwa-buildpack/reference/targetables/TargetableSet/
Verify Targetables section listed on the left navigation with working links to individual reference doc pages

Screenshots / Screen Captures (if appropriate)

Checklist

I have added tests to cover my changes, if necessary.
I have added translations for new strings, if necessary.
I have updated the documentation accordingly, if necessary.

…nd add examples to reference docs

PWAStudioBot · 2021-01-22T16:15:12Z

	Messages
📖	Associated JIRA tickets: PWA-796.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 20020e8

revanth0212

Wonderful work @jcalcaben. Love the way you have structured the docs.

I have provided minor change requests/questions. Let me know what you think of them?

revanth0212 · 2021-01-26T16:59:58Z

packages/pwa-buildpack/lib/WebpackTools/targetables/TargetableESModuleObject.js

     *
-     * @alias TargetableESModuleObject#add
+     * @param {string} importString A static import declaration
+     *


Can we add a @return here?

revanth0212 · 2021-01-26T17:00:04Z

packages/pwa-buildpack/lib/WebpackTools/targetables/TargetableESModuleObject.js

+     * Adds a module or modules to the object using the `addImport()` function.
+     *
+     * @param  {...string} args Static import declaration(s)
+     *


Can we add a @return here?

revanth0212 · 2021-01-26T18:02:56Z

pwa-devdocs/src/pwa-buildpack/extensibility-framework/index.md

+#### Extensions security
+
+For security reasons, PWA Studio restrict the scope of Targetable modifications within an extension's intercept file to source files in the extension package.
+This prevents third party extensions from making source code changes to a storefront project without the developer's knowledge.


Can we elaborate on this? From an extension point of view, what is the targetable scope (its own files, storefront files, other extensions etc)?

revanth0212 · 2021-01-26T18:05:41Z

pwa-devdocs/src/pwa-buildpack/reference/targetables/SingleImportStatement/index.md

+Use this to reference the component's local name when adding custom code with Targetables.
+
+```jsx
+let Button = new SingleImportStatement("Button from './button'");


Suggested change

let Button = new SingleImportStatement("Button from './button'");

let Button = new SingleImportStatement("import Button from './button'");

Fun Fact: You can skip import here and the class is smart enough to infer what you mean.

See: https://github.com/magento/pwa-studio/blob/develop/packages/pwa-buildpack/lib/WebpackTools/targetables/__tests__/SingleImportStatement.spec.js#L34-L46

I'll add that note in the code.

Good to know. Nicee.

revanth0212 · 2021-01-26T18:08:28Z

pwa-devdocs/src/pwa-buildpack/reference/targetables/SingleImportStatement/index.md

+Pass in an import statement to the constructor to create a new `SingleImportStatement` object.
+
+```js
+const useQueryImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");


useQueryImport sounds like a hook/talon.

Suggested change

const useQueryImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");

const queryHookImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");

revanth0212 · 2021-01-26T18:10:51Z

pwa-devdocs/src/pwa-buildpack/reference/targetables/SingleImportStatement/index.md

+Use the `changeBinding()` function to rename the variable bound to the imported object.
+
+```js
+const useQueryImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");


Suggested change

const useQueryImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");

const queryHookImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");

revanth0212 · 2021-01-26T18:11:04Z

pwa-devdocs/src/pwa-buildpack/reference/targetables/SingleImportStatement/index.md

+```js
+const useQueryImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");
+
+const useQueryImport2 = useQueryImport.changeBinding('useQuery2');


Suggested change

const useQueryImport2 = useQueryImport.changeBinding('useQuery2');

const queryHookImport2 = useQueryImport.changeBinding('useQuery2');

revanth0212 · 2021-01-26T18:20:49Z

pwa-devdocs/src/pwa-buildpack/reference/targetables/TargetableESModule/index.md

+
+#### Import statement limits
+
+The `addImport()` function can only handle import statements with a single binding.


It's nice that you have mentioned this. Do we have a plan to handle/support multiple imports? If so, I guess it makes sense to mention that here for curious devs.

I'm not aware of any plans

revanth0212 · 2021-01-26T18:26:03Z

pwa-devdocs/src/pwa-buildpack/reference/targetables/TargetableReactComponent/index.md

+        'div className={pageClass}',
+        '<span>appendJSX succeeded!</span>'
+    )
+        .addJSXClassName('div className={pageClass}', 'pageClass')


Suggested change

.addJSXClassName('div className={pageClass}', 'pageClass')

.addJSXClassName('div className={pageClass}', 'newClass')

revanth0212 · 2021-01-26T18:28:40Z

pwa-devdocs/src/pwa-buildpack/reference/targetables/TargetableSet/index.md

+This class is available as a named import from `@magento/pwa-buildpack`.
+
+```js
+const { Targetables } = require('@magento/pwa-buildpack')


Can you mention that Targetables and TargetableSet are the same because nowhere in the following examples we are using TargetableSet?

revanth0212

Wonderful work. Approved.

dpatil-magento

Approved latest commits.

jcalcaben added 15 commits January 13, 2021 12:27

Add TOC for Targetables reference section

6752aa8

Add TargetableModule reference doc

77aad5e

Add rest of targetable files to reference build script

83845dc

Add topics for rest of Targetable files

8702d38

Add Targetables explainer

4846d0a

Add targetables tutorial overview file

ce3e640

Add overview for how to modify code with Targetables

083112e

Clean up TargetableSet reference and add examples

607de57

Clean up TargetableModule reference docs and add examples

79cac64

Clean up SingleImportStatement reference docs and add examples

a8e384e

Clean up doc blocks in TargetableESModule and SingleImportStatement a…

dfe8e65

…nd add examples to reference docs

Clean up reference docs for TargetableESModuleArray and add example

af12c20

Clean up reference docs for TargetableESModuleObject and add example

727a81f

Add example to TargetableReactComponent

14d2c8a

Add links to reference docs

fcea41d

jcalcaben added pkg:pwa-devdocs documentation This pertains to documentation. version: Patch This changeset includes backwards compatible bug fixes. docs documentation labels Jan 22, 2021

m2-community-project bot added Progress: ready for review labels Jan 22, 2021

PWAStudioBot added the pkg:pwa-buildpack label Jan 22, 2021

Fix linting and prettier issues

e733a50

revanth0212 requested changes Jan 26, 2021

View reviewed changes

m2-community-project bot assigned revanth0212 Jan 26, 2021

m2-community-project bot removed the Progress: ready for review label Jan 26, 2021

m2-community-project bot added the Progress: changes requested label Jan 26, 2021

Add return statements to chainables

ee955e8

m2-community-project bot added Progress: review and removed Progress: changes requested labels Jan 26, 2021

Make changes based on feedback

7aad53d

jcalcaben requested a review from revanth0212 January 26, 2021 22:35

revanth0212 previously approved these changes Jan 27, 2021

View reviewed changes

m2-community-project bot added Progress: approved and removed Progress: review labels Jan 27, 2021

Run prettier

e92511d

jcalcaben dismissed revanth0212’s stale review via e92511d January 27, 2021 17:35

Merge branch 'develop' into jimothy/pwa-796_targetables-docs

20020e8

dpatil-magento self-requested a review January 28, 2021 15:18

dpatil-magento approved these changes Jan 28, 2021

View reviewed changes

m2-community-project bot assigned dpatil-magento Jan 28, 2021

dpatil-magento merged commit cf11495 into develop Jan 28, 2021

dpatil-magento deleted the jimothy/pwa-796_targetables-docs branch January 28, 2021 15:19

m2-community-project bot added Progress: done and removed Progress: approved labels Jan 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Doc] Targetables #2966

[Doc] Targetables #2966

jcalcaben commented Jan 22, 2021

PWAStudioBot commented Jan 22, 2021 •

edited

Loading

revanth0212 left a comment

revanth0212 Jan 26, 2021

revanth0212 Jan 26, 2021

revanth0212 Jan 26, 2021

revanth0212 Jan 26, 2021

jcalcaben Jan 26, 2021

revanth0212 Jan 27, 2021

revanth0212 Jan 26, 2021

revanth0212 Jan 26, 2021

revanth0212 Jan 26, 2021

revanth0212 Jan 26, 2021

jcalcaben Jan 26, 2021

revanth0212 Jan 26, 2021

revanth0212 Jan 26, 2021

revanth0212 left a comment

dpatil-magento left a comment

	let Button = new SingleImportStatement("Button from './button'");
	let Button = new SingleImportStatement("import Button from './button'");

	const useQueryImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");
	const queryHookImport = new SingleImportStatement("import { useQuery } from '@apollo/react-hooks'");

	const useQueryImport2 = useQueryImport.changeBinding('useQuery2');
	const queryHookImport2 = useQueryImport.changeBinding('useQuery2');


		#### Import statement limits

		The `addImport()` function can only handle import statements with a single binding.

	.addJSXClassName('div className={pageClass}', 'pageClass')
	.addJSXClassName('div className={pageClass}', 'newClass')

[Doc] Targetables #2966

[Doc] Targetables #2966

Conversation

jcalcaben commented Jan 22, 2021

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Jan 22, 2021 • edited Loading

revanth0212 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

revanth0212 left a comment

Choose a reason for hiding this comment

dpatil-magento left a comment

Choose a reason for hiding this comment

PWAStudioBot commented Jan 22, 2021 •

edited

Loading