Skip to content

πŸ—οΈ Update no-import lint rule to forbid sub-paths#26531

Merged
jridgewell merged 1 commit intoampproject:masterfrom
jridgewell:update-no-import-rule
Feb 26, 2020
Merged

πŸ—οΈ Update no-import lint rule to forbid sub-paths#26531
jridgewell merged 1 commit intoampproject:masterfrom
jridgewell:update-no-import-rule

Conversation

@jridgewell
Copy link
Copy Markdown
Contributor

Caroline accidentally imported from 'preact/src', and took a bit to figure out what was going wrong. This fix now forbids importing from the preact/src sub-paths.

@jridgewell jridgewell requested a review from rsimha January 28, 2020 23:35
@amp-owners-bot
Copy link
Copy Markdown

Hey @erwinmombay, these files were changed:

  • build-system/eslint-rules/no-import.js

if (name !== forbidden.import) {
continue;
const importSource = forbidden.import;
if (name === importSource || name.startsWith(`${importSource}/`)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you now delete this rule?

{
import: 'preact/hooks',
message:
"Please import preact/hooks from 'src/preact'. This gives us type safety.",
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jridgewell Was this rule still needed? With this change, isn't the preact rule sufficient?

Caroline accidentally imported `from 'preact/src'`, and took a bit to figure out what was going wrong. This fix now forbids importing from the `preact/src` sub-paths.
@jridgewell jridgewell force-pushed the update-no-import-rule branch from 4287642 to 2357800 Compare February 26, 2020 20:27
@jridgewell jridgewell merged commit b0744d3 into ampproject:master Feb 26, 2020
@jridgewell jridgewell deleted the update-no-import-rule branch February 26, 2020 22:00
robinvanopstal added a commit to jungvonmatt/amphtml that referenced this pull request Feb 27, 2020
* master: (54 commits)
  inabox-resources: Minor test improvement (ampproject#26916)
  DocInfo: replace metaTags with viewport in API (ampproject#26687)
  πŸ› SwG now uses AMP sendBeacon interface (ampproject#26970)
  πŸ— Allow array destructuring on preact hooks (ampproject#26901)
  Gulp Dep Check: fail on unused entries (ampproject#26981)
  Update no-import lint rule to forbid sub-paths (ampproject#26531)
  πŸ› amp-ad type blade - fix bladeOnLoad callback (ampproject#26627)
  πŸ“– Clarify when max-age is required (ampproject#26956)
  ♻️ Consolidate players as .i-amphtml-media-component (ampproject#26967)
  Add Preact Enzyme tests (ampproject#26529)
  Fixes `update_tests` flag on `gulp validator` (ampproject#26965)
  πŸ“¦ Update dependency google-closure-library to v20200224 (ampproject#26986)
  πŸ— Transform aliased configured components (ampproject#26541)
  ✨ InaboxResources: Observe intersections for some elements' viewportCallbacks (ampproject#26942)
  variable substitutions: Support allowlist lookup in AmpDocShadow (ampproject#26731)
  cl/297197875 Revision bump for ampproject#26877 (ampproject#26982)
  Json fix (ampproject#26971)
  πŸ“¦ Update dependency mocha to v7.1.0 (ampproject#26976)
  Add documentation for amp-access-scroll (ampproject#26782)
  make controls always shown in amp for email (ampproject#25714)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants