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

chore(agnostic): migrate DOMWorld #6054

Merged
merged 1 commit into from Jun 25, 2020
Merged

chore(agnostic): migrate DOMWorld #6054

merged 1 commit into from Jun 25, 2020

Conversation

jackfranklin
Copy link
Collaborator

DOMWorld only needs to use Node's fs module if you're adding a
filepath as a script/style tag. We can detect this case and run the
require inline such that in a browser this code won't execute.

@mathiasbynens mathiasbynens changed the title chore(agnostic): Migrate DOMWorld chore(agnostic): migrate DOMWorld Jun 19, 2020
@jackfranklin
Copy link
Collaborator Author

@mathisbynens @paullewis PTAL. I'm not sure if this is the best approach or not for this but it's the least friction way to make progress so I'm quite keen to do a first pass using this approach and see how that treats us. Equally welcome to other ideas too.

A follow up PR will start to split out helper.ts so it becomes easier to use promisify in Node contexts.

* limitations under the License.
*/

export const isNode = typeof document === 'undefined';
Copy link
Member

Choose a reason for hiding this comment

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

Not gonna lie, the 15 lines of legalese followed by nothing but this elegant one-liner made me chuckle

Choose a reason for hiding this comment

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

This check fails with JSDOM (e.g. in jest)

DOMWorld only needs to use Node's `fs` module if you're adding a
filepath as a script/style tag. We can detect this case and run the
`require` inline such that in a browser this code won't execute.
@jackfranklin jackfranklin merged commit f6ef805 into main Jun 25, 2020
@jackfranklin jackfranklin deleted the agnostic-dom-world branch June 25, 2020 09:19
mathiasbynens pushed a commit that referenced this pull request Jun 25, 2020
DOMWorld only needs to use Node's `fs` module if you're adding a
filepath as a script/style tag. We can detect this case and run the
`require` inline such that in a browser this code won't execute.
*
* You can pass a URL, filepath or string of contents. Note that when running Puppeteer
* in a browser environment you cannot pass a filepath and should use either
* `url` or `content`.
Copy link

@Hypnosphi Hypnosphi Jul 10, 2020

Choose a reason for hiding this comment

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

I think it makes sense to mention this in changelog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants