-
Notifications
You must be signed in to change notification settings - Fork 10
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
enhancement/Issue-1118: Single File Bundles for SSR and API routes #1186
Conversation
70f1844
to
8fad6f7
Compare
I'm not totally sure if this PR is ready. At the moment, there are greenwood/packages/cli/test/cases/build.default.ssr-static-export/src/pages/artists.js Line 34 in 8fad6f7
If that call does not return an array, the suite fails. I attempted to add some defensive code as a test, which resulted in a single failure of a spec due to a mismatch of values. The code below allows for the process to continue, but it doesn't produce the desired outcome...obviously. const artistsListItems = Array.isArray(artists) ? artists : []
.filter(artist => artist.isActive === '1')
.map((artist) => { When hitting the API directly in the browser, I get the same error I'm seeing from the |
My bad @DevLab2425 🤦 Please see #1188 for the fix 😇 |
a51d550
to
8ee2e48
Compare
@thescientist13 If I isolate a suite of tests, that suite will typically pass. However, anytime I attempt to run the full set of tests, I get errors from various suites outside of the bundling logic I've changed. Here's an example of the failures I'm seeing outside of the As I work through the errors I'm seeing things like in-use ports, missing file imports, etc. I'm just not sure whether what I'm seeing is environmental or code-based due to my changes. Could we setup some time later this week to help me see where my blast zone might be, and what might be a red herring? Also, why type of tests are running with The latest suite I'm chasing is the
🤷 |
const bundle = await rollup(rollupConfig); | ||
await bundle.write(rollupConfig.output); | ||
if (apiConfigs.length > 0 && apiConfigs[0].input.length !== 0) { | ||
apiConfigs.forEach(async rollupConfig => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to do something async
in a loop, we should use a for ... of
or for ... in
instead. forEech
is synchronous and combining these two can lead to unexpected behaviors (something I would like to more thoroughly clean up in #823 )
for (const rollupConfig of apiConfigs) {
const bundle = await rollup(rollupConfig);
await bundle.write(rollupConfig.output);
}
Could you be running other Greenwood project's at the same time by chance, occupying ports One way to confirm if a process is running is run Another tip is to pass the debug flag to runner = new Runner(true); Fwiw, I ran all the tests locally from this branch and only got 8 tests failing, exclusively related to API behaviors, which is also what CI is reporting, so seems like your blast radius is small 💣 😃 3070 passing (5m) 30 pending 8 failing
All test cases are pretty much the same, they run the CLI and validate some output, so in the context of a CLI app I suppose they would be E2E tests? (technically though for something like the static router feature, I would love to run it in a browser). But no, the answer is more boring than that. The
Not related to But then if others fail, I will
I wonder if it could be related to #1186 (comment)? I confirmed that rollup config had an entry when logging |
I have passing tests locally, but the CI is still failing many times. I'll use the CI reports to try to track things down, but having difficulty identifying things without failures locally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, not sure why GitHub Actions is failing so hard here, even after re-running the jobs, but I ran yarn test:exp
locally and all the specs passed. 😕
While i look into that, I think last steps would be to step into each of the adapter plugins and see if we can now remove all the extraneous file copying we had to do.
- Vercel - https://github.com/ProjectEvergreen/greenwood/blob/master/packages/plugin-adapter-vercel/src/index.js#L107
- Netlify - https://github.com/ProjectEvergreen/greenwood/blob/master/packages/plugin-adapter-netlify/src/index.js#L118
Thanks for all this, please keep the faith! 😇
I'm going to play around with this locally a bit to examine the new output and will aim to patch these changes into one of the respective demo repos to do some final validation testing, which I think I can do this weekend. (and also look into the test cases <> GitHub Actions.)
if (rollupConfig.input.length !== 0) { | ||
const bundle = await rollup(rollupConfig); | ||
await bundle.write(rollupConfig.output); | ||
if (apiConfigs.length > 0 && apiConfigs[0].input.length !== 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor tip: but for each of these loops, if we use a for ... of
, we can get access to the index directly.
async function bundleApiRoutes(compilation) {
const apiConfigs = await getRollupConfigForApis(compilation);
if (apiConfigs.length > 0 && apiConfigs[0].input.length !== 0) {
for (const config of apiConfigs) {
const bundle = await rollup(config);
await bundle.write(config.output);
}
}
}
@DevLab2425 TypeError: Failed to parse URL from file://file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/node_modules/lit-html/lit-html.js.map
at new Request (node:internal/deps/undici/undici:5272:19)
at NodeModulesResource.resolve (file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/plugins/resource/plugin-node-modules.js:41:12)
... 5 lines matching cause stack trace ...
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:42:37
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:42:37
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:42:37 {
[cause]: TypeError [ERR_INVALID_URL]: Invalid URL
at new NodeError (node:internal/errors:405:5)
at new URL (node:internal/url:676:13)
at new Request (node:internal/deps/undici/undici:5270:25)
at NodeModulesResource.resolve (file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/plugins/resource/plugin-node-modules.js:41:12)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:44:29
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:42:37
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:42:37
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:42:37
at async file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/packages/cli/src/lifecycles/serve.js:42:37 {
input: 'file://file:///Users/owenbuckley/Workspace/project-evergreen/greenwood/node_modules/lit-html/lit-html.js.map',
code: 'ERR_INVALID_URL'
}
} Taking a look, maybe it's something was doing wrong, or perhaps a regression in Node? What version are you using? Worst case we can pin CI to edit 2: OOOOOOH, I get it now. It's obviously breaking in some way. So this weekend let me make an issue for tracking this bug with edit 1: By changing this line to omit the
That could hint to a package exports / node module resolution change on NodeJS side, so might require a bit more investigation, so let me tinker with this some more and we'll get some way forward on this ASAP. (perhaps time to escalate this issue - #684 . 🤔 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @DevLab2425 , just have some updates to share:
Regarding the build failures, I've opened #1191 to remove the offending code path completely until I can properly fix / implement it later in #684. For local dev, I would just use Node < 18..19.0
I'll plan to get it merged Wednesday evening after the weekly meeting.
I also got around to doing some testing over the weekend of these changes into our Vercel demo repo and it worked great! I even patched in some cleanup of the Vercel adapter to remove a few of the cp
calls we were making and was able to see a definite reduction in file generated. 💪
# before CLI
➜ greenwood-demo-adapter-vercel git:(main) tree public
public
├── 1733327959.ed8fa148.js.map
├── 396762157.af333475.js.map
├── 404.html
├── 758806547.5433709a.js.map
├── 949355521.f4273cc9.js.map
├── __products.js
├── _products.js
├── api
│ ├── assets
│ ├── card.51aef4b7.js
│ ├── fragment.js
│ ├── greeting.js
│ ├── products.c34c47ae.js
│ └── search.js
├── card.51aef4b7.js
├── card.51aef4b7.js.map
├── favicon.ico
├── graph.json
├── index.html
├── manifest.json
├── resources.json
├── search
│ └── index.html
└── styles
└── main.19738545.css
5 directories, 21 files
# after CLI
➜ greenwood-demo-adapter-vercel git:(chore/upgrade-greenwood-0.30.0) ✗ tree public
public
├── 1733327959.ed8fa148.js.map
├── 396762157.af333475.js.map
├── 404.html
├── 758806547.5433709a.js.map
├── 949355521.f4273cc9.js.map
├── __products.js
├── _products.js
├── api
│ ├── assets
│ ├── card.51aef4b7.js
│ ├── fragment.js
│ ├── greeting.js
│ └── search.js
├── card.51aef4b7.js
├── card.51aef4b7.js.map
├── favicon.ico
├── graph.json
├── index.html
├── manifest.json
├── resources.json
├── search
│ └── index.html
└── styles
└── main.19738545.css
5 directories, 20 files
# before adapter
➜ greenwood-demo-adapter-vercel git:(89730d7) ✗ tree .vercel
.vercel
└── output
├── config.json
├── functions
│ ├── api
│ │ ├── fragment.func
│ │ │ ├── assets
│ │ │ ├── card.51aef4b7.js
│ │ │ ├── fragment.js
│ │ │ ├── index.js
│ │ │ ├── package.json
│ │ │ └── products.c34c47ae.js
│ │ ├── greeting.func
│ │ │ ├── assets
│ │ │ ├── card.51aef4b7.js
│ │ │ ├── greeting.js
│ │ │ ├── index.js
│ │ │ ├── package.json
│ │ │ └── products.c34c47ae.js
│ │ └── search.func
│ │ ├── assets
│ │ ├── card.51aef4b7.js
│ │ ├── index.js
│ │ ├── package.json
│ │ ├── products.c34c47ae.js
│ │ └── search.js
│ └── products.func
│ ├── __products.js
│ ├── _products.js
│ ├── card.51aef4b7.js
│ ├── index.js
│ └── package.json
└── static
├── 1733327959.ed8fa148.js.map
├── 396762157.af333475.js.map
├── 404.html
├── 758806547.5433709a.js.map
├── 949355521.f4273cc9.js.map
├── __products.js
├── _products.js
├── api
│ ├── assets
│ ├── card.51aef4b7.js
│ ├── fragment.js
│ ├── greeting.js
│ ├── products.c34c47ae.js
│ └── search.js
├── card.51aef4b7.js
├── card.51aef4b7.js.map
├── favicon.ico
├── graph.json
├── index.html
├── manifest.json
├── resources.json
├── search
│ └── index.html
└── styles
└── main.19738545.css
16 directories, 42 files
# after adapter
➜ greenwood-demo-adapter-vercel git:(chore/upgrade-greenwood-0.30.0) ✗ tree .vercel
.vercel
└── output
├── config.json
├── functions
│ ├── api
│ │ ├── fragment.func
│ │ │ ├── card.51aef4b7.js
│ │ │ ├── fragment.js
│ │ │ ├── index.js
│ │ │ └── package.json
│ │ ├── greeting.func
│ │ │ ├── card.51aef4b7.js
│ │ │ ├── greeting.js
│ │ │ ├── index.js
│ │ │ └── package.json
│ │ └── search.func
│ │ ├── card.51aef4b7.js
│ │ ├── index.js
│ │ ├── package.json
│ │ └── search.js
│ └── products.func
│ ├── __products.js
│ ├── _products.js
│ ├── index.js
│ └── package.json
└── static
├── 1733327959.ed8fa148.js.map
├── 396762157.af333475.js.map
├── 404.html
├── 758806547.5433709a.js.map
├── 949355521.f4273cc9.js.map
├── __products.js
├── _products.js
├── api
│ ├── assets
│ ├── card.51aef4b7.js
│ ├── fragment.js
│ ├── greeting.js
│ └── search.js
├── card.51aef4b7.js
├── card.51aef4b7.js.map
├── favicon.ico
├── graph.json
├── index.html
├── manifest.json
├── resources.json
├── search
│ └── index.html
└── styles
└── main.19738545.css
13 directories, 37 files
One call out is that when using new URL
in a file, a chunk will always be generated as opposed to being "inlined" when it is an import
since that's just how that works. As you can see in the output above, I still had to copy card.js into every API function folder, even though it is technically only needed for two of the APIs.
It would be interesting to see if there's anyway to link these to the entry point that produced it, so we could have a stronger coupling between assets of an API or SSR page. Probably something I can make another ticket for.
After Adapter (Final)
➜ greenwood-demo-adapter-vercel git:(chore/upgrade-greenwood-0.30.0) ✗ tree .vercel
.vercel
└── output
├── config.json
├── functions
│ ├── api
│ │ ├── fragment.func
│ │ │ ├── card.51aef4b7.js
│ │ │ ├── fragment.js
│ │ │ ├── index.js
│ │ │ └── package.json
│ │ ├── greeting.func
│ │ │ ├── greeting.js
│ │ │ ├── index.js
│ │ │ └── package.json
│ │ └── search.func
│ │ ├── card.51aef4b7.js
│ │ ├── index.js
│ │ ├── package.json
│ │ └── search.js
│ └── products.func
│ ├── index.js
│ ├── package.json
│ ├── products.route.chunk.d46126ed.js
│ └── products.route.js
└── static
├── 1733327959.ed8fa148.js.map
├── 396762157.af333475.js.map
├── 404.html
├── 758806547.5433709a.js.map
├── 949355521.f4273cc9.js.map
├── api
│ ├── card.51aef4b7.js
│ ├── fragment.js
│ ├── greeting.js
│ └── search.js
├── card.51aef4b7.js
├── card.51aef4b7.js.map
├── favicon.ico
├── graph.json
├── index.html
├── manifest.json
├── products.route.chunk.d46126ed.js
├── products.route.js
├── resources.json
├── search
│ └── index.html
└── styles
└── main.19738545.css
12 directories, 36 files
OK, #1190 has been merged 👍 |
bc25da9
to
b10655d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, felt to clean up some things here!
Just need to publish some upstream changes for WCC then this will be good to go!
e75ac52
to
785394e
Compare
…1186) * Issue-1118: Refactor rollup config generation for APIs * Issue-1118: Refactor rollup config generation for SSR * Issue-1118: Refactor forEach to use for-in for the ssr config generation * Issue-1118: Convert forEach to for..in * Issue-1118: Remove unused code * refactor away bundling work arounds and add comments * refactor SSR page bundling to avoid hacky entry point placeholder hack * patch custom element registry check from wcc * refactor SSR page output name from .entry to .route * document breaking changes for adapter plugins * refactor import meta relative asset path escaping * refactor API routes and adapters for mapped API bundles * misc refactoring and docs update * latest WCC patches * windows compatibility * update adapter docs example * remove patches --------- Co-authored-by: Owen Buckley <owenbuckley13@gmail.com>
…1186) * Issue-1118: Refactor rollup config generation for APIs * Issue-1118: Refactor rollup config generation for SSR * Issue-1118: Refactor forEach to use for-in for the ssr config generation * Issue-1118: Convert forEach to for..in * Issue-1118: Remove unused code * refactor away bundling work arounds and add comments * refactor SSR page bundling to avoid hacky entry point placeholder hack * patch custom element registry check from wcc * refactor SSR page output name from .entry to .route * document breaking changes for adapter plugins * refactor import meta relative asset path escaping * refactor API routes and adapters for mapped API bundles * misc refactoring and docs update * latest WCC patches * windows compatibility * update adapter docs example * remove patches --------- Co-authored-by: Owen Buckley <owenbuckley13@gmail.com>
…1186) * Issue-1118: Refactor rollup config generation for APIs * Issue-1118: Refactor rollup config generation for SSR * Issue-1118: Refactor forEach to use for-in for the ssr config generation * Issue-1118: Convert forEach to for..in * Issue-1118: Remove unused code * refactor away bundling work arounds and add comments * refactor SSR page bundling to avoid hacky entry point placeholder hack * patch custom element registry check from wcc * refactor SSR page output name from .entry to .route * document breaking changes for adapter plugins * refactor import meta relative asset path escaping * refactor API routes and adapters for mapped API bundles * misc refactoring and docs update * latest WCC patches * windows compatibility * update adapter docs example * remove patches --------- Co-authored-by: Owen Buckley <owenbuckley13@gmail.com>
* feature/discussion 1117 Isolation Mode (v1) (#1206) * isolation mode for SSR pages and API routes for greenwood serve * documentation for isolation mode option and global config test case * misc refactoring * set isolation mode to true for Lit renderer plugin * set isolation mode to true for Lit renderer plugin * enhancement/Issue-1118: Single File Bundles for SSR and API routes (#1186) * Issue-1118: Refactor rollup config generation for APIs * Issue-1118: Refactor rollup config generation for SSR * Issue-1118: Refactor forEach to use for-in for the ssr config generation * Issue-1118: Convert forEach to for..in * Issue-1118: Remove unused code * refactor away bundling work arounds and add comments * refactor SSR page bundling to avoid hacky entry point placeholder hack * patch custom element registry check from wcc * refactor SSR page output name from .entry to .route * document breaking changes for adapter plugins * refactor import meta relative asset path escaping * refactor API routes and adapters for mapped API bundles * misc refactoring and docs update * latest WCC patches * windows compatibility * update adapter docs example * remove patches --------- Co-authored-by: Owen Buckley <owenbuckley13@gmail.com> * v0.30.0-alpha.1 * feature/issue 923 native import attributes for CSS and JSON (#1215) * intial draft of import attributes support for CSS and JSON * all test cases passing * need patch package * wcc patches for import attributes and CSSStylesheet shim * bump min NodeJS version for exp specs * temp disable ESLint * develop based import assertion specs * serve based import attributes specs * add preIntercept resource plugin lifecycle and refactor PostCSS to use it * all test cases passing for import attributes support * refactor built in CSS and JSON intercepting * demo code * raw plugin docs and package.json updates * update latest documentation for custom loaders support in NodeJS * update custom import docs * upgrade wcc v0.13.0 * only need Node 18 for github actions * css imports and raw plugin interop with test cases * lit renderer import attribute test cases and documentation * refactor matchers support for raw plugin instead of patching and add test cases * disable describe.only * update usage for custom resource plugins to showcase usage of import attributes * document preIntercept lifecycle and convert Babel to use it * restore ESLint * enable debug logging for failing specs * refactor theme pack specs * fix linting * remove CSS and JSON packages from being publishable * clean up console logs and comments * rename exp test cases to loadersnaming prefix * fix command in github actions * remove plugin-import-css callout from plugin-postcss README * remove demo code from website * refine PostCSS plugin intercepting * first pass on resource tracking and bundling refactor with lit polyfills removal from CLI * interim work around to solve double rendering and undefined WCC bugs * refactor frontmatter for graph and standard html plugin for SSR * rename templates directory to layouts * refactor over bundling of static script assets in bundleSsrPages * handle bundling styles within bundleSsrPages * post rebase tweaks * custom elements as layouts * post rebase tweaks * WCC patched support for TS pages * support and tests for API routes as a custom dynamic format * restore TS tests and make servePage default * document custom page format support * fix lint * patch latest WCC TypeScript changes * cleanup default app layout content * handle rollup circular dependency warnings for API routes * rename test cases from templates to layout * collapse API routes directory into pages directory * bump to wc-compiler 0.14.0 --------- Co-authored-by: Paul Barry <devlab2425@gmail.com>
…1186) * Issue-1118: Refactor rollup config generation for APIs * Issue-1118: Refactor rollup config generation for SSR * Issue-1118: Refactor forEach to use for-in for the ssr config generation * Issue-1118: Convert forEach to for..in * Issue-1118: Remove unused code * refactor away bundling work arounds and add comments * refactor SSR page bundling to avoid hacky entry point placeholder hack * patch custom element registry check from wcc * refactor SSR page output name from .entry to .route * document breaking changes for adapter plugins * refactor import meta relative asset path escaping * refactor API routes and adapters for mapped API bundles * misc refactoring and docs update * latest WCC patches * windows compatibility * update adapter docs example * remove patches --------- Co-authored-by: Owen Buckley <owenbuckley13@gmail.com>
* feature/discussion 1117 Isolation Mode (v1) (#1206) * isolation mode for SSR pages and API routes for greenwood serve * documentation for isolation mode option and global config test case * misc refactoring * set isolation mode to true for Lit renderer plugin * set isolation mode to true for Lit renderer plugin * enhancement/Issue-1118: Single File Bundles for SSR and API routes (#1186) * Issue-1118: Refactor rollup config generation for APIs * Issue-1118: Refactor rollup config generation for SSR * Issue-1118: Refactor forEach to use for-in for the ssr config generation * Issue-1118: Convert forEach to for..in * Issue-1118: Remove unused code * refactor away bundling work arounds and add comments * refactor SSR page bundling to avoid hacky entry point placeholder hack * patch custom element registry check from wcc * refactor SSR page output name from .entry to .route * document breaking changes for adapter plugins * refactor import meta relative asset path escaping * refactor API routes and adapters for mapped API bundles * misc refactoring and docs update * latest WCC patches * windows compatibility * update adapter docs example * remove patches --------- Co-authored-by: Owen Buckley <owenbuckley13@gmail.com> * v0.30.0-alpha.1 * feature/issue 923 native import attributes for CSS and JSON (#1215) * intial draft of import attributes support for CSS and JSON * all test cases passing * need patch package * wcc patches for import attributes and CSSStylesheet shim * bump min NodeJS version for exp specs * temp disable ESLint * develop based import assertion specs * serve based import attributes specs * add preIntercept resource plugin lifecycle and refactor PostCSS to use it * all test cases passing for import attributes support * refactor built in CSS and JSON intercepting * demo code * raw plugin docs and package.json updates * update latest documentation for custom loaders support in NodeJS * update custom import docs * upgrade wcc v0.13.0 * only need Node 18 for github actions * css imports and raw plugin interop with test cases * lit renderer import attribute test cases and documentation * refactor matchers support for raw plugin instead of patching and add test cases * disable describe.only * update usage for custom resource plugins to showcase usage of import attributes * document preIntercept lifecycle and convert Babel to use it * restore ESLint * enable debug logging for failing specs * refactor theme pack specs * fix linting * remove CSS and JSON packages from being publishable * clean up console logs and comments * rename exp test cases to loadersnaming prefix * fix command in github actions * remove plugin-import-css callout from plugin-postcss README * remove demo code from website * refine PostCSS plugin intercepting * first pass on resource tracking and bundling refactor with lit polyfills removal from CLI * interim work around to solve double rendering and undefined WCC bugs * refactor frontmatter for graph and standard html plugin for SSR * rename templates directory to layouts * refactor over bundling of static script assets in bundleSsrPages * handle bundling styles within bundleSsrPages * post rebase tweaks * custom elements as layouts * post rebase tweaks * WCC patched support for TS pages * support and tests for API routes as a custom dynamic format * restore TS tests and make servePage default * document custom page format support * fix lint * patch latest WCC TypeScript changes * cleanup default app layout content * handle rollup circular dependency warnings for API routes * rename test cases from templates to layout * collapse API routes directory into pages directory * bump to wc-compiler 0.14.0 --------- Co-authored-by: Paul Barry <devlab2425@gmail.com>
Related Issue
Resolves #1118
Summary of Changes
URL
so we can bundle instead of the hacky string replace solutionTODO
URL
, otherwise it would just be inlined, fwiwpostinstall