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(vite-plugin-angular): add support for JIT mode #374

Merged
merged 8 commits into from
May 9, 2023

Conversation

brandonroberts
Copy link
Member

@brandonroberts brandonroberts commented May 5, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Which package are you modifying?

  • vite-plugin-angular
  • vite-plugin-nitro
  • astro-angular
  • create-analog
  • router
  • platform
  • content
  • nx-plugin
  • trpc

What is the current behavior?

Closes #368

What is the new behavior?

Enabling JIT mode through the Analog Platform and Vite Angular Plugin is an option.

/// <reference types="vitest" />

import { defineConfig } from 'vite';
import { offsetFromRoot } from '@nx/devkit';
import angular from '@analogjs/vite-plugin-angular';

// https://vitejs.dev/config/
export default defineConfig(({ mode }) => {
  return {
    root: 'src',
    plugins: [
      angular({ jit: boolean }), // enabled automatically when running tests with Vitest
    ],
    test: {
      globals: true,
      environment: 'jsdom',
      setupFiles: ['src/test-setup.ts'],
      include: ['**/*.spec.ts'],
    },
    define: {
      'import.meta.vitest': mode !== 'production',
    },
  };
});

When running tests, JIT mode is used instead of AOT which provides faster compilation of files, while still supporting internal and external styles and templates. This will speed up compilation time with libraries such as Vitest, Playwright, Storybook, etc.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@netlify
Copy link

netlify bot commented May 5, 2023

Deploy Preview for analog-blog ready!

Name Link
🔨 Latest commit 5e758c9
🔍 Latest deploy log https://app.netlify.com/sites/analog-blog/deploys/645a3df36f05640007fff125
😎 Deploy Preview https://deploy-preview-374--analog-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 5, 2023

Deploy Preview for analog-docs ready!

Name Link
🔨 Latest commit 5e758c9
🔍 Latest deploy log https://app.netlify.com/sites/analog-docs/deploys/645a3df35a03850008e2aeb4
😎 Deploy Preview https://deploy-preview-374--analog-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 5, 2023

Deploy Preview for analog-app ready!

Name Link
🔨 Latest commit 5e758c9
🔍 Latest deploy log https://app.netlify.com/sites/analog-app/deploys/645a3df35a03850008e2aeaf
😎 Deploy Preview https://deploy-preview-374--analog-app.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@brandonroberts
Copy link
Member Author

@sand4rt this should help with Playwright compilation speed also.

@sand4rt
Copy link
Contributor

sand4rt commented May 6, 2023

@brandonroberts Awesome! Gonna try this asap!

@@ -31,6 +31,6 @@ export function platformPlugin(opts: Options = {}): Plugin[] {
(platformOptions.ssr
? devServerPlugin({ entryServer: opts.entryServer })
: false) as Plugin,
...angular(opts?.vite),
...angular({ jit: platformOptions.jit, ...opts?.vite }),

Choose a reason for hiding this comment

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

If opts?.vite ends up being undefined this spread will error. I think you want ...(opts?.vite ?? {})?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks

});

return {
code: result?.code || '',
Copy link
Contributor

Choose a reason for hiding this comment

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

minor (non-blocking): Wouldn't make sense to use the ?? instead of || here?

}: {
inlineStylesExtension: string;
}): Plugin {
let styleTransform: PluginContainer['transform'] | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: if we have a utils library/functions somewhere we could add a Utility type like this:
type ExtractMethodSignature<T extends Record<any, any>, K> = T[K];

and this way you can change this to:
let styleTransform: ExtractMethodSignature<PluginContainer, 'transform'> | undefined;

This would make only sense if we do this many times in our code. Maybe there is a better TS solution for this.

Copy link

@marcus-sa marcus-sa May 9, 2023

Choose a reason for hiding this comment

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

@gergobergo the correct TS solution is already being used. There's no point in abstracting something that doesn't need to be abstracted.

@brandonroberts brandonroberts merged commit 07af493 into main May 9, 2023
18 checks passed
@brandonroberts brandonroberts deleted the feat-jit-mode branch May 9, 2023 12:58
@sand4rt
Copy link
Contributor

sand4rt commented May 9, 2023

@brandonroberts thought the templateUrl would still work with angular({ jit: true }) somehow but unfortunately that is not the case. I wonder if this is still a useful feature for testing because of this?

@brandonroberts
Copy link
Member Author

@sand4rt it supports templateUrl and styleUrls with Vitests. I'll have to check and see what the issue is with Playwright. Will you open a new issue with a sample repo?

@KlausEverWalkingDev
Copy link

KlausEverWalkingDev commented May 11, 2023

After running some tests I came with these weirdly conclusions:
image

@brandonroberts
Copy link
Member Author

@KlausEverWalkingDev this is the same as disabling jit mode altogether. The vite object overrides the analog plugin options for the Vite plugin.

@KlausEverWalkingDev
Copy link

KlausEverWalkingDev commented May 11, 2023

Oh, thanks. I added it while reading the analog source files, but I admit I didn't understand the real meaning of it. By the way, what's the difference between analog and angular plugin? I didn't get when to use only one of them or both.

@brandonroberts
Copy link
Member Author

Oh, thanks. I added it while reading the analog source files, but I admit I didn't understand the real meaning of it. By the way, what's the difference between analog and angular plugin? I didn't get when to use only one of them or both.

The analog plugin is the "glue" between the analog packages.

The angular plugin is what handles connecting Angular compilation to Vite. It can be used on its own and is used for Astro, Qwik, and other Vite integration

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.

Feature: Add support for JIT mode for testing
7 participants