zspecza / common-tags

🔖 Useful template literal tags for dealing with strings in ES2015+
Other
1.99k stars 60 forks source link

`codeBlock` breaking change adds newlines #215

Closed alecgibson closed 2 years ago

alecgibson commented 2 years ago

Our build just got bumped from v1.8.0 to v1.8.1.

We have some strings defined like:

const html = codeBlock`
  <div>foo</div>
`;

In v1.8.0, this renders as '<div>foo</div>', but v1.8.1 trims the leading newline, whilst preseverving the trailing newline: '<div>foo</div>\n'.

I'm assuming this is something to do with bumping an upstream dependency, since that's the only thing that's really changed.

Is this behaviour expected? If so, a major version bump would have been nice. If not, this is a bug, and should probably have some tests added somewhere?

Here's a MWE: https://runkit.com/alecgibson/618e478004cdf60008330c6a

fatfisz commented 2 years ago

I'm positive this is not due to a dependency bump in common-tags, since common-tags has no dependencies at all! package-lock.json only includes dev dependencies, which are not shipped with the package.

For now I can only suggest pinning the version to 1.8.0. I'm not sure where the change comes from and I can't investigate it right now.

alecgibson commented 2 years ago

In that case, maybe some upstream change affected the build...?

alumni commented 2 years ago

@fatfisz I think that 1.8.0 was released from the wrong branch (from master) 😄

So now we got all the breaking changes are between master and release, including stripIndent using trimResultTransformer('smart') which is not what 1.8.0 was doing.

At least if you look at the 1.8.0 code on npm, the 'smart' option was not available: https://cdn.jsdelivr.net/npm/common-tags@1.8.0/es/trimResultTransformer/trimResultTransformer.js https://cdn.jsdelivr.net/npm/common-tags@1.8.1/es/trimResultTransformer/trimResultTransformer.js

fatfisz commented 2 years ago

@alumni Thanks a lot! This was the hint I needed 😔

The branches were alright, but I must've run the wrong command and the build step didn't happen. Now I ran npm run release, which also did npm run prerelease, which in turn caused npm run build to happen.

I checked https://cdn.jsdelivr.net/npm/common-tags@1.8.2/es/trimResultTransformer/trimResultTransformer.js to make sure and it seems to be back to normal.

@alecgibson Please check with your team if everything's back to normal. Sorry for the problem.

alumni commented 2 years ago

@fatfisz I can confirm that the new release is fine.

alecgibson commented 2 years ago

Yup looks like that's done the trick. Thanks for fixing!