Skip to content
This repository was archived by the owner on Jan 22, 2019. It is now read-only.

fix: prevent transitive importing of entrypoints#184

Merged
chrismwendt merged 1 commit intomasterfrom
prevent-transitive-imports-of-entrypoints
Sep 18, 2018
Merged

fix: prevent transitive importing of entrypoints#184
chrismwendt merged 1 commit intomasterfrom
prevent-transitive-imports-of-entrypoints

Conversation

@chrismwendt
Copy link

This prevents accidentally importing one entrypoint in another, which was the root cause of #175

This works by tagging each entrypoint in webpack with window.EXTENSION_ENV = <name of entrypoint>, having each entrypoint call assertEnv(<name of entrypoint>), and throwing an error if window.EXTENSION_ENV !== <name of entrypoint>.

This also removes existing violations of these assertions ✨

@chrismwendt chrismwendt requested review from ijsnow and sqs September 18, 2018 01:01
@chrismwendt chrismwendt force-pushed the prevent-transitive-imports-of-entrypoints branch from f883cdd to 8c9c2af Compare September 18, 2018 01:02
Copy link

@ijsnow ijsnow left a comment

Choose a reason for hiding this comment

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

Nice

@@ -1,10 +1,10 @@
// We want to polyfill first.
Copy link

Choose a reason for hiding this comment

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

Ah, was this the entry point for the extensions page? Just confused about the name change and don't remember this file before.

Copy link
Author

Choose a reason for hiding this comment

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

It might have been the entrypoint a while back, but currently the extensions page is part of the options page, so this entrypoint is unnecessary.

import { createExtensionsContextController } from '../../../shared/backend/extensions'
import { GQL } from '../../../types/gqlschema'
import { sourcegraphUrl } from '../../util/context'
import { BrowserSettingsEditor } from './browserSettingsEditor'
Copy link

Choose a reason for hiding this comment

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

Should the file name in this import be UpperCamelCase?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, fixed.

export const onFirstMessage = (port: chrome.runtime.Port, callback: (message: any) => void) => {
const cb = message => {
port.onMessage.removeListener(cb)
callback(message)
Copy link

Choose a reason for hiding this comment

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

Nifty logic. Wouldn't have thought of that.

@chrismwendt chrismwendt force-pushed the prevent-transitive-imports-of-entrypoints branch from 8c9c2af to ef47228 Compare September 18, 2018 02:15
@chrismwendt chrismwendt merged commit ff75251 into master Sep 18, 2018
@chrismwendt chrismwendt deleted the prevent-transitive-imports-of-entrypoints branch September 18, 2018 02:38
@sourcegraph-bot
Copy link

🎉 This PR is included in version 1.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants