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

Make durationInFrames optional #621

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/core/src/sequencing/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ export const SequenceContext = createContext<SequenceContextType | null>(null);
export type SequenceProps = {
children: React.ReactNode;
from: number;
durationInFrames: number;
durationInFrames?: number;
name?: string;
layout?: 'absolute-fill' | 'none';
showInTimeline?: boolean;
};

export const Sequence: React.FC<SequenceProps> = ({
from,
durationInFrames,
durationInFrames = Infinity,
children,
name,
layout = 'absolute-fill',
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/test/nested-sequences.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ test('Negative offset edge case', () => {
const endAt = 90;

const content = (
<Sequence from={40} durationInFrames={Infinity}>
<Sequence from={40}>
<Sequence from={0 - startFrom} durationInFrames={endAt}>
<NestedChild />
</Sequence>
Expand Down
7 changes: 0 additions & 7 deletions packages/core/src/test/sequence-validation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ import {expectToThrow} from './expect-to-throw';

describe('Composition-validation render should throw with invalid props', () => {
describe('Throw with invalid duration props', () => {
test('It should throw if Sequence has missing duration', () => {
expectToThrow(
// @ts-expect-error
() => render(<Sequence from={0} />),
/You passed to durationInFrames an argument of type undefined, but it must be a number./
);
});
test('It should throw if Sequence has non-integer durationInFrames', () => {
expectToThrow(
() =>
Expand Down
8 changes: 4 additions & 4 deletions packages/docs/components/SequenceExamples/SequenceForward.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,24 @@ const BlueSquare: React.FC = () => {

const DelayExample: React.FC = () => {
return (
<Sequence from={30} durationInFrames={Infinity}>
<Sequence from={30} >
<BlueSquare/>
</Sequence>
);
};

const TrimStartExample: React.FC = () => {
return (
<Sequence from={-15} durationInFrames={Infinity}>
<Sequence from={-15} >
<BlueSquare />
</Sequence>
);
};

const TrimAndDelayExample: React.FC = () => {
return (
<Sequence from={30} durationInFrames={Infinity}>
<Sequence from={-15} durationInFrames={Infinity}>
<Sequence from={30} >
<Sequence from={-15} >
<BlueSquare />
</Sequence>
</Sequence>
Expand Down
14 changes: 7 additions & 7 deletions packages/docs/docs/sequence.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ The Sequence component is a high order component and accepts, besides it's child

- `from` _(required)_: At which frame it's children should assume the video starts. When the sequence is at `frame`, it's children are at frame `0`.

- `durationInFrames` _(required)_: For how many frames the sequence should be displayed. Children are unmounted if they are not within the time range of display. If you don't want to limit the duration of the sequence, you can pass `Infinity`.
- `durationInFrames` _(optional)_: For how many frames the sequence should be displayed. Children are unmounted if they are not within the time range of display. By default it will be `Infinity` to avoid limit the duration of the sequence.

- `name` _(optional)_: You can give your sequence a name and it will be shown as the label of the sequence in the timeline of the Remotion preview. This property is purely for helping you keep track of sequences in the timeline.

Expand Down Expand Up @@ -47,7 +47,7 @@ const MyVideo = () => {

### Delay

If you would like to delay the content by say 30 frames, you can wrap it in <br/> `<Sequence from={30} durationInFrames={Infinity}>`.
If you would like to delay the content by say 30 frames, you can wrap it in <br/> `<Sequence from={30}>`.

<SequenceForwardExample type="delay" />
<br />
Expand All @@ -58,7 +58,7 @@ import {Sequence} from 'remotion'
// ---cut---
const MyVideo = () => {
return (
<Sequence from={30} durationInFrames={Infinity}>
<Sequence from={30} >
<BlueSquare />
</Sequence>
)
Expand All @@ -76,7 +76,7 @@ In this example, we wrap the square in `<Sequence from={0} durationInFrames={45}
### Trim start

To trim the start of some content, we can pass a negative value to `from`.
In this example, we wrap the square in `<Sequence from={-15} durationInFrames={Infinity}>` and as a result, the animation has already progressed by 15 frames at the start of the video.
In this example, we wrap the square in `<Sequence from={-15}>` and as a result, the animation has already progressed by 15 frames at the start of the video.

<SequenceForwardExample type="trim-start" />

Expand All @@ -94,8 +94,8 @@ import {Sequence} from 'remotion'
// ---cut---
const TrimAndDelayExample: React.FC = () => {
return (
<Sequence from={30} durationInFrames={Infinity}>
<Sequence from={-15} durationInFrames={Infinity}>
<Sequence from={30}>
<Sequence from={-15}>
<BlueSquare />
</Sequence>
</Sequence>
Expand All @@ -110,4 +110,4 @@ See the [`<Series />`](/docs/series) helper component, which helps you calculate
## See also

- [Reuse components using Sequences](/docs/reusability)
- [`<Series /> `](/docs/series)
- [`<Series />`](/docs/series)
2 changes: 1 addition & 1 deletion packages/docs/docs/sequences.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const MyVideo = () => {
<Sequence from={0} durationInFrames={40}>
<Title title="Hello" />
</Sequence>
<Sequence from={40} durationInFrames={Infinity}>
<Sequence from={40}>
<Title title="World" />
</Sequence>
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/docs/docs/series.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ The `<Series />` component takes no props may only contain a list of `<Series.Se

This component is a high order component, and accepts, besides it's children, the following props:

- `durationInFrames` _(required)_: For how many frames the sequence should be displayed. Children are unmounted if they are not within the time range of display. If you don't want to limit the duration of the sequence, you can pass `Infinity`.
- `durationInFrames` _(optional)_: For how many frames the sequence should be displayed. Children are unmounted if they are not within the time range of display. By default it will be `Infinity` to avoid limit the duration of the sequence.

- `offset`: _(optional)_: Pass a positive number to delay the beginning of the sequence. Pass a negative number to start the sequence earlier, and to overlay the sequence with the one that comes before.

Expand Down
2 changes: 1 addition & 1 deletion packages/docs/docs/use-current-frame.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const MyVideo = () => {
return (
<div>
<Title />
<Sequence from={10} durationInFrames={Infinity}>
<Sequence from={10}>
<Subtitle />
</Sequence>
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/docs/docs/using-audio.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const MyVideo = () => {
return (
<div>
<div>Hello World!</div>
<Sequence from={100} durationInFrames={Infinity}>
<Sequence from={100}>
<Audio src={audio} />
</Sequence>
</div>
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const getRules = (typescript: boolean) => {
"@remotion/deterministic-randomness": "warn",
"@remotion/no-string-assets": "warn",
"@remotion/even-dimensions": "warn",
"@remotion/duration-in-frames": "warn",
"@typescript-eslint/explicit-module-boundary-types": "off",
};
};
Expand Down
3 changes: 2 additions & 1 deletion packages/eslint-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"description": "A set of rules helping you avoid common pitfalls in Remotion.",
"scripts": {
"build": "tsc -d",
"test": "jest"
"test": "jest",
"watch": "tsc -w"
},
"main": "dist/index.js",
"bugs": {
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ import evenDimensions from "./rules/even-dimensions";
import nomp4Import from "./rules/no-mp4-import";
import noStringAssets from "./rules/no-string-assets";
import warnNativeMediaTag from "./rules/warn-native-media-tag";
import durationInFrames from "./rules/no-duration-frames-infinity";

const rules = {
"no-mp4-import": nomp4Import,
"warn-native-media-tag": warnNativeMediaTag,
"deterministic-randomness": deterministicRandomness,
"no-string-assets": noStringAssets,
"even-dimensions": evenDimensions,
"duration-in-frames": durationInFrames
};

export = {
Expand Down
83 changes: 83 additions & 0 deletions packages/eslint-plugin/src/rules/no-duration-frames-infinity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { ESLintUtils } from "@typescript-eslint/experimental-utils";

const createRule = ESLintUtils.RuleCreator((name) => {
return `https://github.com/remotion-dev/remotion`;
});

type Options = [];

type MessageIds = "DurationInFrames";

const DurationInFrames =
"Infinity is now the default, so you can remove the prop.";

export default createRule<Options, MessageIds>({
name: "duration-in-frames",
meta: {
type: "problem",
docs: {
description: DurationInFrames,
category: "Best Practices",
recommended: "warn",
},
fixable: "code",
schema: [],
messages: {
DurationInFrames,
},
},
defaultOptions: [],
create: (context) => {
return {
JSXAttribute: (node) => {
if (node.type !== "JSXAttribute") {
return;
}

if (node.name.name !== "durationInFrames") {
return;
}
const value = node.value;
if (!value || value.type !== "JSXExpressionContainer") {
return;
}

const expression = value.expression;
if (!expression || expression.type !== "Identifier") {
return;
}

const stringValue = expression.name;
if (typeof stringValue !== "string") {
return;
}

const parent = node.parent;
if (!parent) {
return;
}

if (parent.type !== "JSXOpeningElement") {
return;
}

const name = parent.name;
if (name.type !== "JSXIdentifier") {
return;
}
if (name.name !== "Sequence") {
return;
}
if (stringValue === "Infinity") {
context.report({
messageId: "DurationInFrames",
node,
fix: (fixer) => {
return fixer.removeRange([node.name.range[0], value.range[1]]);
},
});
}
},
};
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { ESLintUtils } from "@typescript-eslint/experimental-utils";
import rule from "../rules//no-duration-frames-infinity";

const ruleTester = new ESLintUtils.RuleTester({
parser: "@typescript-eslint/parser",
parserOptions: {
ecmaFeatures: {
jsx: true,
},
},
});

ruleTester.run("no-duration-frames-infinity", rule, {
valid: [
`
import {Sequence} from 'remotion';

export const Re = () => {
return (
<Sequence durationInFrames={1}>
Hi
</Sequence>
);
}
`,
],
invalid: [
{
code: `
import {Composition} from 'remotion';

export const Re = () => {
return (
<Sequence>
khalid283 marked this conversation as resolved.
Show resolved Hide resolved
Hi
</Sequence>
);
}
`,
errors: [
{
messageId: "DurationInFrames",
},
],
},
],
});
2 changes: 1 addition & 1 deletion packages/renderer/src/test/asset-calculation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ test('Should calculate volumes correctly', async () => {
test('Should calculate startFrom correctly', async () => {
const assetPositions = await getPositions(() => {
return (
<Sequence from={1} durationInFrames={Infinity}>
<Sequence from={1}>
<Audio
startFrom={100}
endAt={200}
Expand Down