Add PWAManifestTransformer #3404

Drapegnik · 2019-08-14T21:56:47Z

↪️ Pull Request

Resolves #3362, Resolves #235, Resolves #301

✔️ PR Todo

ported WebManifestAsset -> PWAManifestTransformer
support .json webmanifest
Added/updated unit tests for this change
Included links to related issues/PRs

packages/core/integration-tests/test/webmanifest.js

Drapegnik · 2019-08-14T22:43:48Z

Hi, @devongovett! Could you check this please?
If everything is ok, I'll add the rest.

Should I add something for rewriting urls?

packages/transformers/webmanifest/src/dependencies.js

devongovett · 2019-08-15T04:00:10Z

Looking good so far @Drapegnik! 😄

packages/transformers/webmanifest/src/dependencies.js

packages/transformers/pwa-manifest/src/PWAManifestTransformer.js

Drapegnik · 2019-08-15T12:12:38Z

@devongovett,

I decided to divide WebManifestTransformer to PWA & WebExtension transformers, because manifests have different structure, so:

everything under rel="manifest marked as env: {context: 'pwa-manifest'}
manifest.json without pwa-manifest context is webext-manifest

packages/core/integration-tests/test/pwa-manifest.js

Drapegnik · 2019-08-15T12:24:38Z

packages/transformers/pwa-manifest/src/PWAManifestTransformer.js

+import nullthrows from 'nullthrows';
+
+const getSrcHandler = (opt = {}) => (asset, dep) => {
+ dep.src = asset.addURLDependency(dep.src, opt);


I add this for urls rewriting and get following from this manifest:

{ "name": "pwa-webmanifest-example", "icons": [ { "src": "1eda49598769bd3a3cd2b1e31d3cfaff", "sizes": "100x100", "type": "image/png" } ], "screenshots": [ { "src": "f432622196df181e7734bdfd95fa62d0", "sizes": "100x100", "type": "image/png" }, { "src": "" } ], "serviceworker": { "src": "d771eacd0fc5e521bf134585524d4727" } }

Yep, those ids will be replaced later with urls once the final bundle names are known.

How can I test it?

yarn build parceldev build --no-cache packages/core/integration-tests/test/integration/pwa-manifest/inde x.html

produces the same file (with ids)

@devongovett @DeMoorJasper @padmaia @wbinnssmith,

Could you tell me where is this final replacement (ids -> urls) occurs?
Looks like .webmanifest is ignored.

never mind, I found - generateDepToBundlePath)

the problem was because .webmanifest was being processed by RawPackager, which returns stream, so typeof contents !== 'string' and generateDepToBundlePath was skipped

I fixed this by adding StringPackager - not sure if this is the best way

devongovett · 2019-08-15T17:39:20Z

packages/configs/default/index.json

@@ -5,6 +5,7 @@
 "@parcel/transformer-babel",
 "@parcel/transformer-js"
 ],
+ "manifest.json": ["@parcel/transformer-webext-manifest"],


This seems dangerous to me. Many things could be called manifest.json in theory. I'd say, keep the web extension transformer as an official plugin, but don't include in the default config. That way, if you're writing a web extension, you can opt-in to the processing of manifest.json, and it won't affect other apps.

So, I add this logic to json-transformer:

// handle webmanifest .json if (asset.env.context === 'pwa-manifest') { asset.type = 'webmanifest'; return [asset]; }

Drapegnik · 2019-08-16T20:20:22Z

Ready to final review.
I'd like to add WebextManifestTransformer in next PR

Drapegnik · 2019-08-20T21:16:34Z

@devongovett, hi!
Could you please review this pr?

Drapegnik · 2019-09-20T10:35:17Z

@devongovett, hey?)

sygint · 2019-12-05T10:44:16Z

@Drapegnik looks like it has conflicts now, but I'd love to see @devongovett merge this. This will allow me to move many projects over to parcel and greatly reduce my complexity for my builds

Drapegnik · 2019-12-06T17:01:37Z

@gnarmedia, I'm also really looking forward to some response from @devongovett.

Drapegnik · 2019-12-06T17:03:25Z

packages/transformers/html/src/dependencies.js

@@ -129,7 +129,8 @@ export default function collectDependencies(

 if (tag === 'link' && attrs.rel === 'manifest' && attrs.href) {
 attrs.href = asset.addURLDependency(attrs.href, {
- isEntry: true
+ isEntry: true,
+ env: {context: 'pwa-manifest'}


I did rebase and now get:

Error: Received URL without a pathname.

something with moduleSpecifier (new api?)

How do I properly add manifest dep here?

DeMoorJasper · 2020-01-15T12:44:21Z

@Drapegnik could you rebase this on v2 and fix the failing tests? Lemme know if you need any help with fixing tests

Drapegnik · 2020-01-22T21:14:32Z

@DeMoorJasper, hi! after rebase I've got:

  pwa-manifest
    1) should support .webmanifest
    ✓ should support .json webmanifest (87ms)
    2) should treat .webmanifest as an entry module so it doesn't get content hashed
    3) should rename webmanifest *.json to *.webmanifest

with Error: Received URL without a pathname . could you help me with it?

maybe it's due . webmanifest extension? (json manifest somehow passed)

We're current not including this in the HTML file yet since parcel has an issue bundling a JSON resouce from HTML. There is a PR that looks to resolve this issue (parcel-bundler/parcel#3404)

DeMoorJasper · 2020-03-12T15:55:29Z

@Drapegnik apologies for not reaching out sooner and giving feedback on this plugin.

I've tried to get the tests to pass a couple times, but also couldn't figure out why they were failing and deviated too much from this PR in my attempts to fix it to give proper feedback, so I never really got around to writing a structured review to get you going and eventually moved on to other issues...

@mischnic apparently is working on fixing the issues with your PR, not entirely sure if we should close this one and move progress over to that PR?

Drapegnik · 2020-03-12T17:10:17Z

@DeMoorJasper, ok, but what about #3439, which depends on this one?

DeMoorJasper · 2020-03-12T17:46:59Z

@Drapegnik I assume that one will not be included in @mischnic 's PR

Drapegnik force-pushed the transformer-webmanifest branch from 2f25bbb to 7b72ca7 Compare August 14, 2019 22:13

Drapegnik commented Aug 14, 2019

View reviewed changes

packages/core/integration-tests/test/webmanifest.js Outdated Show resolved Hide resolved

padmaia added the ✨ Parcel 2 label Aug 14, 2019

Drapegnik force-pushed the transformer-webmanifest branch from 7b72ca7 to f8ab22f Compare August 14, 2019 22:42

devongovett reviewed Aug 15, 2019

View reviewed changes

packages/transformers/webmanifest/src/dependencies.js Outdated Show resolved Hide resolved

devongovett reviewed Aug 15, 2019

View reviewed changes

packages/transformers/webmanifest/src/dependencies.js Outdated Show resolved Hide resolved

Drapegnik commented Aug 15, 2019

View reviewed changes

packages/transformers/pwa-manifest/src/PWAManifestTransformer.js Outdated Show resolved Hide resolved

Drapegnik commented Aug 15, 2019

View reviewed changes

packages/transformers/pwa-manifest/src/PWAManifestTransformer.js Outdated Show resolved Hide resolved

Drapegnik commented Aug 15, 2019

View reviewed changes

packages/core/integration-tests/test/pwa-manifest.js Outdated Show resolved Hide resolved

Drapegnik commented Aug 15, 2019

View reviewed changes

devongovett reviewed Aug 15, 2019

View reviewed changes

Drapegnik changed the title ~~[WIP] Add WebManifestTransformer~~ [WIP] Add PWAManifestTransformer Aug 15, 2019

Drapegnik force-pushed the transformer-webmanifest branch from 4fba6bf to 73d0213 Compare August 16, 2019 11:57

mischnic added the 🔌 Plugins label Aug 16, 2019

Drapegnik changed the title ~~[WIP] Add PWAManifestTransformer~~ Add PWAManifestTransformer Aug 16, 2019

flybayer mentioned this pull request Aug 19, 2019

Parcel's development server not working with plugin kevincharm/parcel-plugin-web-extension#23

Open

Drapegnik mentioned this pull request Aug 19, 2019

[WIP] Add WebextManifestTransformer #3439

Closed

2 tasks

Drapegnik requested a review from devongovett September 20, 2019 10:34

Drapegnik force-pushed the transformer-webmanifest branch 2 times, most recently from 24cd209 to 5bc521c Compare December 6, 2019 17:00

Drapegnik commented Dec 6, 2019

View reviewed changes

Drapegnik added 4 commits January 22, 2020 23:50

Add basic WebManifestTransformer

434db2a

Add separate PWA & webext transformers

369a6b2

Refactor pwa-manifest-transformer, made collectDependencies pure

e5ef671

Remove transformer-webext-manifest from default config

4dbf21b

Drapegnik force-pushed the transformer-webmanifest branch from 5bc521c to e052b33 Compare January 22, 2020 20:51

Add StringPackager to fix .webmanifest deps url replacing

ec02278

Drapegnik force-pushed the transformer-webmanifest branch from e052b33 to ec02278 Compare January 22, 2020 21:09

Drapegnik requested a review from DeMoorJasper January 22, 2020 21:14

Merge branch 'v2' into transformer-webmanifest

64a7f5f

VolkerSchiewe mentioned this pull request Feb 16, 2020

Parcel 2 Integration VolkerSchiewe/trumpet#6

Closed

Merge branch 'v2' into transformer-webmanifest

334d1c5

mischnic mentioned this pull request Mar 11, 2020

Process webmanifests #4313

Closed

DeMoorJasper closed this Mar 21, 2020

ranisalt mentioned this pull request Jan 22, 2021

Add transformer for web app manifests #5700

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PWAManifestTransformer #3404

Add PWAManifestTransformer #3404

Drapegnik commented Aug 14, 2019 •

edited

Loading

Drapegnik commented Aug 14, 2019

devongovett commented Aug 15, 2019

Drapegnik commented Aug 15, 2019

Drapegnik Aug 15, 2019

devongovett Aug 15, 2019

Drapegnik Aug 15, 2019

Drapegnik Aug 16, 2019

Drapegnik Aug 16, 2019

Drapegnik Aug 16, 2019

devongovett Aug 15, 2019

Drapegnik Aug 15, 2019 •

edited

Loading

Drapegnik commented Aug 16, 2019

Drapegnik commented Aug 20, 2019

Drapegnik commented Sep 20, 2019

sygint commented Dec 5, 2019

Drapegnik commented Dec 6, 2019

Drapegnik Dec 6, 2019

DeMoorJasper commented Jan 15, 2020

Drapegnik commented Jan 22, 2020

DeMoorJasper commented Mar 12, 2020 •

edited

Loading

Drapegnik commented Mar 12, 2020

DeMoorJasper commented Mar 12, 2020 •

edited

Loading

Add PWAManifestTransformer #3404

Add PWAManifestTransformer #3404

Conversation

Drapegnik commented Aug 14, 2019 • edited Loading

↪️ Pull Request

✔️ PR Todo

Drapegnik commented Aug 14, 2019

devongovett commented Aug 15, 2019

Drapegnik commented Aug 15, 2019

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

Drapegnik Aug 15, 2019 • edited Loading

Choose a reason for hiding this comment

Drapegnik commented Aug 16, 2019

Drapegnik commented Aug 20, 2019

Drapegnik commented Sep 20, 2019

sygint commented Dec 5, 2019

Drapegnik commented Dec 6, 2019

Choose a reason for hiding this comment

DeMoorJasper commented Jan 15, 2020

Drapegnik commented Jan 22, 2020

DeMoorJasper commented Mar 12, 2020 • edited Loading

Drapegnik commented Mar 12, 2020

DeMoorJasper commented Mar 12, 2020 • edited Loading

Drapegnik commented Aug 14, 2019 •

edited

Loading

Drapegnik Aug 15, 2019 •

edited

Loading

DeMoorJasper commented Mar 12, 2020 •

edited

Loading

DeMoorJasper commented Mar 12, 2020 •

edited

Loading