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

Refactor Sources #114

Merged
merged 1 commit into from
Apr 29, 2016
Merged

Conversation

jasonLaster
Copy link
Contributor

@jasonLaster jasonLaster commented Apr 28, 2016

Started working on Call Stacks and saw an opportunity to cleanup Sources. Nothing scary here.

  • add _updateText formatting function for sources
  • cleanup sources component

@jasonLaster
Copy link
Contributor Author

Nice -> Tests doing what they're supposed to do:
https://circleci.com/gh/jasonLaster/debugger.html/67

return Object.assign({}, source, { pathname, filename });
}

function getPathnameFromUrl(sourceUrl) {
if (!sourceUrl) {
return "";
}

const url = new URL(sourceUrl);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm convinced now that that should live somewhere else and we should make a selector that extracts the filename from a source. The URL is undefined in node and none of this code should depend on the browser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, that makes a lot of sense. We could do a simple feature check if (URL) {} else {}

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's a selector and used from a component, I think it's ok to use URL and assume it's a browser environment. We don't test components in node, we'll run them in the browser, from what I understand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't test components in node, we'll run them in the browser

Yea, we can decide to do that. Lets wait and see though, it might be simpler or significantly faster to run in node. Not sure yet.

selectors

I'll move it over now

@jasonLaster jasonLaster force-pushed the sources3 branch 2 times, most recently from c9d47b3 to 760f083 Compare April 29, 2016 18:12
+ add _updateText formatting function for sources
+ cleanup sources component
@jlongster
Copy link
Contributor

+1 mergable!

@jasonLaster jasonLaster merged commit cd0ebfe into firefox-devtools:master Apr 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants