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.

Sign up for GitHub

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

Jump to bottom

coerce rollup ids with query string to consistent line separators to avoid query strings breaking rollup #621

Merged
merged 5 commits into from
Jun 4, 2021
Merged

coerce rollup ids with query string to consistent line separators to avoid query strings breaking rollup #621

merged 5 commits into from
Jun 4, 2021

Conversation

thescientist13
Copy link
Member

@thescientist13 thescientist13 commented May 31, 2021 •
edited
Loading

Related Issue

resolves #620 and is a regression introduced as part of #615 (doesn't impact users yet and should be done before #615 ). Also refer to #606

Summary of Changes

  1. Coerce all import statements in Rollup (when handling ids with query string to consistent path separator
  2. Have Rollup throw an Error if we have an unresolved import within our own tests or website build

TODO

  1. Right now this is happening in one of our specs but because we only log Rollup warnings, we should make this warning an error during our CI process to try and catch this early (but not break users) 
    '/assets/data.json?type=json' is imported by src\scripts\main.js, but could not be resolved – treating it as an external dependency

@thescientist13 thescientist13 added bug Something isn't working website Tasks related to the projects website / documentation CLI todos discussion tied to an ongoing discussion or meeting notes labels May 31, 2021
@thescientist13 thescientist13 self-assigned this May 31, 2021
@thescientist13 thescientist13 mentioned this pull request May 31, 2021
Closed
5 tasks
@thescientist13 thescientist13 changed the title coerce ids with query string to consitent line seperators coerce rollup ids with query string to consistent line separators to avoid query strings breaking rollup May 31, 2021
This was referenced May 31, 2021
Merged
Merged
@thescientist13 thescientist13 added P0 Critical issue that should get addressed ASAP chore unit testing, maintenance, etc and removed todos labels May 31, 2021
@thescientist13 thescientist13 marked this pull request as ready for review May 31, 2021 19:25
@thescientist13 thescientist13 removed their assignment May 31, 2021
@thescientist13 thescientist13 merged commit 38c9244 into master Jun 4, 2021
@thescientist13 thescientist13 deleted the bug/issue-620-cross-platform-resolve-rollup-query-string-support branch June 4, 2021 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working chore unit testing, maintenance, etc CLI discussion tied to an ongoing discussion or meeting notes P0 Critical issue that should get addressed ASAP website Tasks related to the projects website / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

import JSON via ESM for the website is not bundling correctly on Windows
1 participant
@thescientist13