Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Convert sources reducer to ES modules #2736

Merged
merged 5 commits into from
Apr 25, 2017
Merged

Convert sources reducer to ES modules #2736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b9c71c
Convert reducers/sources to es modules
virzen Apr 25, 2017
68caabf
Convert tests for reducers/sources to es modules
virzen Apr 25, 2017
3a2ecc3
Remove type for sources reducer's state
virzen Apr 25, 2017
52d8650
Set sources reducer's state argument's type to any
virzen Apr 25, 2017
cdb2db1
Use Record notation for sources reducer's argument type
virzen Apr 25, 2017
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
50 changes: 19 additions & 31 deletions src/reducers/sources.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
* @module reducers/sources
*/

const I = require("immutable");
const makeRecord = require("../utils/makeRecord");
const { getPrettySourceURL } = require("../utils/source");
const { prefs } = require("../utils/prefs");
import * as I from "immutable";
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably use import I from "immutable";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems not to be the case:

src/reducers/sources.js:11
 11: import I from "immutable";
            ^ Default import from `immutable`. This module has no default export.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh. Thanks for correction

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh that's interesting. I wonder if this means we should only be importing the things we need from immutable in the future. Thanks for pointing this out and looking it up. 🥂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although there would be no significant gain in size (webpack 1.x bundles whole file anyway) I agree it's a good practice to do so. Also, it would be great for the eventual migration to webpack 2 (which features tree-shaking). 😄

import makeRecord from "../utils/makeRecord";
import { getPrettySourceURL } from "../utils/source";
import { prefs } from "../utils/prefs";

import type { Source, Location } from "../types";
import type { Action } from "../actions/types";
Expand All @@ -34,7 +34,7 @@ export type SourcesState = {
tabs: I.List<any>
};

const State = makeRecord(
export const State = makeRecord(
({
sources: I.Map(),
selectedLocation: undefined,
Expand All @@ -44,7 +44,10 @@ const State = makeRecord(
}: SourcesState)
);

function update(state = State(), action: Action): Record<SourcesState> {
export function update(
state: Record<SourcesState> = State(),
action: Action
): Record<SourcesState> {
let availableTabs = null;
let location = null;

Expand Down Expand Up @@ -270,33 +273,33 @@ function getNewSelectedSourceId(state: SourcesState, availableTabs): string {
// (right now) to type those wrapped functions.
type OuterState = { sources: Record<SourcesState> };

function getSource(state: OuterState, id: string) {
export function getSource(state: OuterState, id: string) {
return state.sources.sources.get(id);
}

function getSourceByURL(state: OuterState, url: string) {
export function getSourceByURL(state: OuterState, url: string) {
return state.sources.sources.find(source => source.get("url") == url);
}

function getSourceById(state: OuterState, id: string) {
export function getSourceById(state: OuterState, id: string) {
return state.sources.sources.find(source => source.get("id") == id);
}

function getSources(state: OuterState) {
export function getSources(state: OuterState) {
return state.sources.sources;
}

function getSourceText(state: OuterState, id: ?string) {
export function getSourceText(state: OuterState, id: ?string) {
if (id) {
return state.sources.sourcesText.get(id);
}
}

function getSourceTabs(state: OuterState) {
export function getSourceTabs(state: OuterState) {
return state.sources.tabs.filter(tab => getSourceByURL(state, tab));
}

function getSelectedSource(state: OuterState) {
export function getSelectedSource(state: OuterState) {
const selectedLocation = state.sources.selectedLocation;
if (!selectedLocation) {
return;
Expand All @@ -307,34 +310,19 @@ function getSelectedSource(state: OuterState) {
);
}

function getSelectedLocation(state: OuterState) {
export function getSelectedLocation(state: OuterState) {
return state.sources.selectedLocation;
}

function getPendingSelectedLocation(state: OuterState) {
export function getPendingSelectedLocation(state: OuterState) {
return state.sources.pendingSelectedLocation;
}

function getPrettySource(state: OuterState, id: string) {
export function getPrettySource(state: OuterState, id: string) {
const source = getSource(state, id);
if (!source) {
return;
}

return getSourceByURL(state, getPrettySourceURL(source.get("url")));
}

module.exports = {
State,
update,
getSource,
getSourceByURL,
getSourceById,
getSources,
getSourceText,
getSourceTabs,
getSelectedSource,
getSelectedLocation,
getPendingSelectedLocation,
getPrettySource
};
6 changes: 3 additions & 3 deletions src/reducers/tests/sources.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
declare var describe: (name: string, func: () => void) => void;
declare var it: (desc: string, func: () => void) => void;

const { State, update } = require("../sources");
const { foobar } = require("../../test/fixtures");
import { State, update } from "../sources";
import { foobar } from "../../test/fixtures";
const fakeSources = foobar.sources.sources;
const expect = require("expect.js");
import expect from "expect.js";

describe("sources reducer", () => {
it("should work", () => {
Expand Down